v4: Flexbox carousel options #21400
Merged
v4: Flexbox carousel options #21400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This revamps the indicators to use flexbox instead of inline-block for added flexbility (hah). Indicators now automatically scale based on the number of elements present, and max out at the `$carousel-indicator-width` instead of always being that wide.
- Drops the absolute positioning of the icons within the left/right controls. We have to keep the controls themselves positioned though since we're overlapping HTML elements here. - No more position, left, right, or margins involved; just some justify-content and align-items. - Add some comments for explaining which flex property-value pair does what. - Remove the unapplied font and line-height stuff now that we're all SVGs and flexbox here.
houndci-bot
reviewed
Dec 22, 2016
| @@ -61,6 +61,10 @@ | |||
| position: absolute; | |||
| top: 0; | |||
| bottom: 0; | |||
| // Use flex for alignment (1-3) | |||
| display: flex; // 1. allow flex styles | |||
| justify-content: center; // 2. horizontally center contents | |||
There was a problem hiding this comment.
Properties should be ordered position, top, bottom, display, align-items, justify-content, width, color, text-align, opacity
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following up on #21389, this implements flexbox into some aspects of the carousel. The entire thing isn't flexbox (yet?) as I think that might require a rethinking of the JS approach. (Compare our JS methods to https://blog.madewithenvy.com/the-order-property-flexbox-carousels-87a168567820#.7vubbda6s, for example.)
However, this does make the following changes:
Sets
display: flexinstead ofblockon active carousel items. There's no visual different here, but if folks want to get creative, this might make it a bit easier.Rebuilds carousel controls (the left and right click areas) with flexbox. They're still absolutely positioned themselves, but the icons within are now aligned via flexbox. Far simpler styling overall.
Rebuilds carousel indicators with flexbox. Instead of a fixed
width, we're now using amax-widthand some flexbox styling to scale to n number of indicators quite easily. This required adding some new horizontalmargin(display: flexdoesn't "suffer" from the same HTML space asinline-block); I've added a new variable to help customize that here.Removed some unused CSS that was left in since the move back to SVGs for icons and more.
Fixes #21305.