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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
76d7f2f
check for data-interval on the first slide of carousel (#31716)
mitchellfyi Oct 2, 2020
49f7dfe
add updateInterval method for elements of a carousel
mitchellfyi Oct 2, 2020
a4d758d
expand carousel unit test to cover initial slide element
mitchellfyi Oct 2, 2020
fa9c367
fix carousel interval test
mitchellfyi Oct 2, 2020
8476aa8
Merge branch 'v4-dev' into carousel-data-bug-v4
XhmikosR Oct 2, 2020
c22485d
Update .bundlewatch.config.json
XhmikosR Oct 2, 2020
d85e48a
update activeElement as soon as slide has finished (before transition…
mitchellfyi Oct 3, 2020
09b832f
Merge branch 'v4-dev' into carousel-data-bug-v4
XhmikosR Nov 1, 2020
d9b167b
set activeElement on carousel slide
mitchellfyi Nov 4, 2020
4f36e31
update carousel tests
mitchellfyi Nov 4, 2020
47b724a
Merge branch 'v4-dev' into carousel-data-bug-v4
mitchellfyi Nov 4, 2020
4ef42c0
Merge branch 'v4-dev' into carousel-data-bug-v4
XhmikosR Nov 5, 2020
a69ced8
Merge branch 'v4-dev' into carousel-data-bug-v4
XhmikosR Nov 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .bundlewatch.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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

@mitchellfyi mitchellfyi Oct 5, 2020 •
edited
Loading

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
View file
Open in desktop
Original file line number Diff line number Diff line change
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