EM-1242: Why CB Success Section (ESB) #124
Conversation
to adjust for the padding that the rest of the content lives within this fixes the buy box being 20px outside of the area that the rest of the content is in see https://trello.com/c/OzxNjQma/34-buy-box-is-out-of-alignment-with-content-area and https://cb-content-enablement.atlassian.net/browse/EM-1480
b/c flag needs to not quite be on the edge
…opic/EM-1237-why-cb
| @@ -222,6 +222,101 @@ $massive-button-horizontal-padding: $base-input-font-size * 2; | |||
| } | |||
| } | |||
|
|
|||
| // Fancy Play Button | |||
| // Initial state | |||
| $ripple-diameter: 24em; | |||
There was a problem hiding this comment.
Should we be putting these in the variables file, @arelia?
There was a problem hiding this comment.
no, going forward this is something I'd prefix with an _ and leave it in the file where it's used
…evelopment/employer-style-base into topic/EM-1242-WCB-Success
| $ripple-diameter: 24em; | ||
| $play-button-size: 4em; | ||
| $ripple-color: white; | ||
| $ripple-hover-color: #447777; |
There was a problem hiding this comment.
I think I chose this color myself. Ask Justin what color he'd like to use from our defined colors
There was a problem hiding this comment.
Will verify with him when he gets in tomorrow.
| $play-button-size: 4em; | ||
| $ripple-color: white; | ||
| $ripple-hover-color: #447777; | ||
| $ripple-hover-transition: 1500ms ease; |
There was a problem hiding this comment.
take a look at the animation documentation and consider how this should be documented ther
- should
$ripple-hover-transition,$ripple-duration, and$ripple-countgo in the variables/animations style sheet? - should generic variables be defined in animations.scss that are then used in place of the number values here?
There was a problem hiding this comment.
Looking over the stylesheet, I agree that the variables mentioned should go into that sheet. I went ahead and moved those, but am using the namespace video-play-button-ripple to better identify those variables.
Regarding your second comment, because there is so little in the animation sheet as of now, I think keeping the values as they are would make more sense. Considering that these values, currently, are very specific to this feature.
| @@ -222,6 +222,101 @@ $massive-button-horizontal-padding: $base-input-font-size * 2; | |||
| } | |||
| } | |||
|
|
|||
| // Fancy Play Button | |||
There was a problem hiding this comment.
this is all so different from everything else in button.scss. I'd leave it in this folder so that it gets documented with buttons in the pattern library, but I'd make a separate file for it. we can work on documentation together
arelia
force-pushed
the
topic/EM-1237-why-cb
branch
from
3510aea to
9b9ce45
Compare
|
@arelia This is ready for your review again (excluding asking Justin about the color of the play button) |
…loyer-style-base into topic/EM-1242-WCB-Success
mrfantasticwonders
changed the base branch from
topic/EM-1237-why-cb
to
master
|
Base branch for this PR has been updated to |
Purpose
This creates a mixin based off @arelia 's animated play button.
https://codepen.io/arelia/pen/wdWwym
JIRA
https://cb-content-enablement.atlassian.net/browse/EM-1242
Changes
Improvements and fixes
Changes to developer setup/environment
Architectural changes
play-rippling-buttonto_button.scssMigrations or Steps to Take on Production
Library changes
Side effects
Screenshots
Before
After
Feature Server
http://web.employer-6.development.c66.me/company/why-careerbuilder
How to Verify These Changes
Specific pages to visit
Steps to take
Responsive considerations
Relevant PRs/Dependencies
Additional Information
N/A