-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Carousel crossfade > Prevent the background to be shown when transitioning #27529
Carousel crossfade > Prevent the background to be shown when transitioning #27529
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can clearly experience an improved crossfade on your demo. Awesome, will definetly use it in some of my projects soonish. JS code looks flawlessly on first glance and CSS appears tidy, too.
Thanks @midzer! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine to me 👍
Can you add a unit test, which show we handle transition-delay
too please ?
@@ -1006,7 +1006,8 @@ $carousel-control-icon-width: 20px !default; | |||
$carousel-control-prev-icon-bg: str-replace(url("data:image/svg+xml,%3csvg xmlns='http://www.w3.org/2000/svg' fill='#{$carousel-control-color}' viewBox='0 0 8 8'%3e%3cpath d='M5.25 0l-4 4 4 4 1.5-1.5-2.5-2.5 2.5-2.5-1.5-1.5z'/%3e%3c/svg%3e"), "#", "%23") !default; | |||
$carousel-control-next-icon-bg: str-replace(url("data:image/svg+xml,%3csvg xmlns='http://www.w3.org/2000/svg' fill='#{$carousel-control-color}' viewBox='0 0 8 8'%3e%3cpath d='M2.75 0l-1.5 1.5 2.5 2.5-2.5 2.5 1.5 1.5 4-4-4-4z'/%3e%3c/svg%3e"), "#", "%23") !default; | |||
|
|||
$carousel-transition: transform .6s ease !default; // Define transform transition first if using multiple transitions (e.g., `transform 2s ease, opacity .5s ease-out`) | |||
$carousel-transition-duration: .6s !default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is only added so that we get the value in JS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And to prevent the transition of the carousel item that is "fading out". We need this because otherwise the classes are removed immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if we could use the transition shorthand, that's why I'm asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can use the shorthand to combine these lines indeed:
https://github.com/twbs/bootstrap/blob/9b3c66258997c10f8bdb1ea521ed64e3775da39d/scss/_carousel.scss#L100-L101
I'll look into that
But I'll still need the $carousel-transition-duration
option as a separate variable.
29aca1f
to
168c422
Compare
@MartijnCuppens so this seems to break on IE 10. See for example https://travis-ci.org/twbs/bootstrap/jobs/448234618#L633 |
When the carousel is transitioning, the background is shown through the images. This PR prevents this from happening by disabling the transition of the carousel item that is fading out and raising the
z-index
of the carousel item that is fading in. I also needed to raise thez-index
of the carousel controls, the indicators had az-index
which was high enough.I had to add the
transition-delay
to thegetTransitionDurationFromElement
function to prevent the classes to be removed too soon. Not sure if we should rename this function to make this clear or not.Issue:
https://getbootstrap.com/docs/4.1/components/carousel/#crossfade
Demo new carousel:
https://deploy-preview-27529--twbs-bootstrap4.netlify.com/docs/4.1/components/carousel/#crossfade