Skip to content
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

Check for data-interval on the first slide of carousel - v4 #31820

Merged
merged 13 commits into from Nov 12, 2020
2 changes: 1 addition & 1 deletion .bundlewatch.config.json
Expand Up @@ -26,7 +26,7 @@
},
{
"path": "./dist/js/bootstrap.bundle.js",
"maxSize": "47.5 kB"
"maxSize": "48 kB"
},
{
"path": "./dist/js/bootstrap.bundle.min.js",
Expand Down
28 changes: 21 additions & 7 deletions js/src/carousel.js
Expand Up @@ -169,6 +169,12 @@ class Carousel {
}

if (this._config.interval && !this._isPaused) {
this._activeElement = this._activeElement || this._element.querySelector(SELECTOR_ACTIVE_ITEM)

if (this._activeElement) {
this._updateInterval(this._activeElement)
}

this._interval = setInterval(
(document.visibilityState ? this.nextWhenVisible : this.next).bind(this),
this._config.interval
Expand Down Expand Up @@ -401,6 +407,17 @@ class Carousel {
}
}

_updateInterval(element) {
const elementInterval = parseInt(element.getAttribute('data-interval'), 10)

if (elementInterval) {
this._config.defaultInterval = this._config.defaultInterval || this._config.interval
this._config.interval = elementInterval
} else {
this._config.interval = this._config.defaultInterval || this._config.interval
}
}

_slide(direction, element) {
const activeElement = this._element.querySelector(SELECTOR_ACTIVE_ITEM)
const activeElementIndex = this._getItemIndex(activeElement)
Expand Down Expand Up @@ -461,13 +478,7 @@ class Carousel {
$(activeElement).addClass(directionalClassName)
$(nextElement).addClass(directionalClassName)

const nextElementInterval = parseInt(nextElement.getAttribute('data-interval'), 10)
if (nextElementInterval) {
this._config.defaultInterval = this._config.defaultInterval || this._config.interval
this._config.interval = nextElementInterval
} else {
this._config.interval = this._config.defaultInterval || this._config.interval
}
this._updateInterval(nextElement)

const transitionDuration = Util.getTransitionDurationFromElement(activeElement)

Expand All @@ -492,6 +503,9 @@ class Carousel {
$(this._element).trigger(slidEvent)
}

// does not wait for the transition to complete
this._activeElement = nextElement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this change please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR is currently out of date (so I reverted it to draft). See #31818


if (isCycling) {
this.cycle()
}
Expand Down
7 changes: 6 additions & 1 deletion js/tests/unit/carousel.js
Expand Up @@ -480,7 +480,7 @@ $(function () {
})

QUnit.test('should set interval from data attribute on individual carousel-item', function (assert) {
assert.expect(2)
assert.expect(3)
var templateHTML = '<div id="myCarousel" class="carousel slide" data-interval="1814">' +
'<div class="carousel-inner">' +
'<div class="carousel-item active" data-interval="2814">' +
Expand Down Expand Up @@ -516,6 +516,11 @@ $(function () {
'</div>'
var $carousel = $(templateHTML)

$carousel.appendTo('body')
$carousel.bootstrapCarousel(0)
assert.strictEqual($carousel.data('bs.carousel')._config.interval, 2814)
$carousel.remove()

$carousel.appendTo('body')
$carousel.bootstrapCarousel(1)
assert.strictEqual($carousel.data('bs.carousel')._config.interval, 3814)
Expand Down