Refactor carousel #21298
Merged
Refactor carousel #21298
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
- Un-nests .carousel-item - Appends .carousel-item- to .left, .right, .next, and .prev - Chain .carousel-item to .active where approproriate
- fix naming of left/right controls - drop the absolute positioning of things and rely on only 3d transforms - remove img styles and require classes to avoid inline-block line-height stuff
This was referenced Dec 5, 2016
…rol width as margins to ensure content doesn't overlap
houndci-bot
reviewed
Dec 5, 2016
| padding-left: 0; | ||
| margin-left: -($carousel-indicators-width / 2); | ||
| // Use the .carousel-control's width as margin so we don't overlay those | ||
| margin-right: $carousel-control-width; |
There was a problem hiding this comment.
Properties should be ordered position, right, bottom, left, z-index, padding-left, margin-right, margin-left, text-align, list-style
|
Pushed a ton more updates to this since opening the PR. The only remaining thing this doesn't address yet is a flexbox variant. I'll tackle that tomorrow I think. Things that might benefit from flexbox:
Some of this likely can be done with utilities, too. |
|
Hello, I was briefly looking at the code change and just happened to find a small typo in the For prev/next controls, `.carousel-control.right` and `.carousel-control.left`
are now `.carousel-control-left` and `.carousel-control-left`,
meaning they no longer require a specific base class.Thanks for this great framework :) |
|
Punting flexbox variant to another PR. Opened #21305 to track. |
This was referenced Dec 6, 2016
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.
This PR refactors the carousel to un-nest CSS selectors, simplify overall styling, and improve documentation of key features. More specifically:
New visuals for a simpler design. I've removed the gradients on the controls, changed the indicators from circles to squares (with larger tap areas), and added new icons.
Un-nest CSS. Instead of
.carousel > .itemand the like, we have.carousel-*prefixed on all classes (save for the.activestate). This includes the slides, controls, and indicators. Fixes Consider clarifying some of the carousel classes (right, left, icon-*) #18396.Simply CSS for sliding effects. We added transforms later in v3's lifecycle, so we had to support transitions for
positionandtransform. No more; now we just have 3D transforms, so our compiled CSS is much simpler.More reliance on utilities. Image styles for within carousel items have been removed, so now you'll need to use utilities or custom styles (e.g.,
.d-blockand.img-fluid). Similarly, I've included an example of responsive captions in our docs built with utilities.Improved documentation. The original example is now split across a handful of examples, each building on the previous (from slides only, to controls, to indicators, to captions). Rewrites some of the intro docs and also provides better guidance on responsive captions and more.
Tweak JS to better handle directional states. This pulls in the JS changes from Add prefix to carousel classes #18830 to accomplish that.
Fixes left carousel-control and right carousel-control elements are not visible in the carousel example #17878, correcting the Carousel example.
Update migration docs to mention changes.