Do not call next when the carousel or the parent isn't visible #23524
Conversation
| if (!document.hidden) { | ||
| // or the carousel or it's parent isn't visible | ||
| if (!document.hidden && | ||
| ($(this._element).is(':visible') && $(this._element).css('visibility') !== 'hidden')) { |
There was a problem hiding this comment.
Are you sure those 2 checks aren't identical?
There was a problem hiding this comment.
Yes the first one just check if the parent or the carousel don't have display: none;
There was a problem hiding this comment.
For more informations and precise explanations (with a better English 😆) : https://api.jquery.com/hidden-selector/
|
So we just need a test with both cases and we are all set. Thanks for the fast patch! |
Johann-S
force-pushed
the
v4-carousel-visibility
branch
from
c5189a6
to
3f78769
Compare
|
I added the two unit tests so @XhmikosR if you're available for a quick code review 😄 |
| setTimeout(function () { | ||
| assert.ok($firstItem.hasClass('active')) | ||
| $carousel.bootstrapCarousel('dispose') | ||
| $parent.attr('style', 'visibility: hidden;') |
There was a problem hiding this comment.
This sets visibility:hidden but doesn't remove display:none, right?
There was a problem hiding this comment.
It remove the display: none; because we set the attribute value no matter what we had before that
There was a problem hiding this comment.
OK, looks good to me then. If you can't think of anything else, please merge :)
Work in progress...Close : #23523