diff options
author | GeoSot <geo.sotis@gmail.com> | 2022-04-21 22:42:17 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-21 22:42:17 +0300 |
commit | 554736834dd929263ab307593f4d4d3f28f61479 (patch) | |
tree | 084f79f074b2e2276ee39ff7ba7b0eb5c4234ad3 | |
parent | 584600bda36ac13ea325617783216d6c6a331c08 (diff) |
Carousel: Fix not used option (`ride`), simplify `cycle` method (#35983)
* Fix not used option (`ride`) (according to docs), continuing of #35753 a247fe9
* separate concept of `programmatical cycle` vs `maybe cycle after click` functionality
-rw-r--r-- | js/src/carousel.js | 72 | ||||
-rw-r--r-- | js/tests/unit/carousel.spec.js | 209 | ||||
-rw-r--r-- | site/content/docs/5.1/components/carousel.md | 8 |
3 files changed, 143 insertions, 146 deletions
diff --git a/js/src/carousel.js b/js/src/carousel.js index 7a30beb10e..64f38d7e64 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -71,19 +71,19 @@ const KEY_TO_DIRECTION = { const Default = { interval: 5000, keyboard: true, - slide: false, pause: 'hover', - wrap: true, - touch: true + ride: false, + touch: true, + wrap: true } const DefaultType = { interval: '(number|boolean)', keyboard: 'boolean', - slide: '(boolean|string)', + ride: '(boolean|string)', pause: '(string|boolean)', - wrap: 'boolean', - touch: 'boolean' + touch: 'boolean', + wrap: 'boolean' } /** @@ -96,13 +96,16 @@ class Carousel extends BaseComponent { this._interval = null this._activeElement = null - this._stayPaused = false this._isSliding = false this.touchTimeout = null this._swipeHelper = null this._indicatorsElement = SelectorEngine.findOne(SELECTOR_INDICATORS, this._element) this._addEventListeners() + + if (this._config.ride === CLASS_NAME_CAROUSEL) { + this.cycle() + } } // Getters @@ -136,30 +139,32 @@ class Carousel extends BaseComponent { this._slide(ORDER_PREV) } - pause(event) { - if (!event) { - this._stayPaused = true - } - + pause() { if (this._isSliding) { triggerTransitionEnd(this._element) - this.cycle(true) } this._clearInterval() } - cycle(event) { - if (!event) { - this._stayPaused = false - } - + cycle() { this._clearInterval() - if (this._config.interval && !this._stayPaused) { - this._updateInterval() + this._updateInterval() - this._interval = setInterval(() => this.nextWhenVisible(), this._config.interval) + this._interval = setInterval(() => this.nextWhenVisible(), this._config.interval) + } + + _maybeEnableCycle() { + if (!this._config.ride) { + return + } + + if (this._isSliding) { + EventHandler.one(this._element, EVENT_SLID, () => this.cycle()) + return } + + this.cycle() } to(index) { @@ -175,8 +180,6 @@ class Carousel extends BaseComponent { const activeIndex = this._getItemIndex(this._getActive()) if (activeIndex === index) { - this.pause() - this.cycle() return } @@ -205,8 +208,8 @@ class Carousel extends BaseComponent { } if (this._config.pause === 'hover') { - EventHandler.on(this._element, EVENT_MOUSEENTER, event => this.pause(event)) - EventHandler.on(this._element, EVENT_MOUSELEAVE, event => this.cycle(event)) + EventHandler.on(this._element, EVENT_MOUSEENTER, () => this.pause()) + EventHandler.on(this._element, EVENT_MOUSELEAVE, () => this._maybeEnableCycle()) } if (this._config.touch && Swipe.isSupported()) { @@ -237,7 +240,7 @@ class Carousel extends BaseComponent { clearTimeout(this.touchTimeout) } - this.touchTimeout = setTimeout(event => this.cycle(event), TOUCHEVENT_COMPAT_WAIT + this._config.interval) + this.touchTimeout = setTimeout(() => this._maybeEnableCycle(), TOUCHEVENT_COMPAT_WAIT + this._config.interval) } const swipeConfig = { @@ -331,12 +334,10 @@ class Carousel extends BaseComponent { return } - this._isSliding = true - const isCycling = Boolean(this._interval) - if (isCycling) { - this.pause() - } + this.pause() + + this._isSliding = true this._setActiveIndicatorElement(nextElementIndex) this._activeElement = nextElement @@ -420,12 +421,6 @@ class Carousel extends BaseComponent { } data[config]() - return - } - - if (data._config.interval && data._config.ride) { - data.pause() - data.cycle() } }) } @@ -449,15 +444,18 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_SLIDE, function (e if (slideIndex) { carousel.to(slideIndex) + carousel._maybeEnableCycle() return } if (Manipulator.getDataAttribute(this, 'slide') === 'next') { carousel.next() + carousel._maybeEnableCycle() return } carousel.prev() + carousel._maybeEnableCycle() }) EventHandler.on(window, EVENT_LOAD_DATA_API, () => { diff --git a/js/tests/unit/carousel.spec.js b/js/tests/unit/carousel.spec.js index 8875f3f003..feaa3f5fe0 100644 --- a/js/tests/unit/carousel.spec.js +++ b/js/tests/unit/carousel.spec.js @@ -63,6 +63,20 @@ describe('Carousel', () => { expect(carouselByElement._element).toEqual(carouselEl) }) + it('should start cycling if `ride`===`carousel`', () => { + fixtureEl.innerHTML = '<div id="myCarousel" class="carousel slide" data-bs-ride="carousel"></div>' + + const carousel = new Carousel('#myCarousel') + expect(carousel._interval).not.toBeNull() + }) + + it('should not start cycling if `ride`!==`carousel`', () => { + fixtureEl.innerHTML = '<div id="myCarousel" class="carousel slide" data-bs-ride="true"></div>' + + const carousel = new Carousel('#myCarousel') + expect(carousel._interval).toBeNull() + }) + it('should go to next item if right arrow key is pressed', () => { return new Promise(resolve => { fixtureEl.innerHTML = [ @@ -95,6 +109,40 @@ describe('Carousel', () => { }) }) + it('should ignore keyboard events if data-bs-keyboard=false', () => { + fixtureEl.innerHTML = [ + '<div id="myCarousel" class="carousel slide" data-bs-keyboard="false">', + ' <div class="carousel-inner">', + ' <div class="carousel-item active">item 1</div>', + ' <div id="item2" class="carousel-item">item 2</div>', + ' </div>', + '</div>' + ].join('') + + spyOn(EventHandler, 'trigger').and.callThrough() + const carouselEl = fixtureEl.querySelector('#myCarousel') + // eslint-disable-next-line no-new + new Carousel('#myCarousel') + expect(EventHandler.trigger).not.toHaveBeenCalledWith(carouselEl, 'keydown.bs.carousel', jasmine.any(Function)) + }) + + it('should ignore mouse events if data-bs-pause=false', () => { + fixtureEl.innerHTML = [ + '<div id="myCarousel" class="carousel slide" data-bs-pause="false">', + ' <div class="carousel-inner">', + ' <div class="carousel-item active">item 1</div>', + ' <div id="item2" class="carousel-item">item 2</div>', + ' </div>', + '</div>' + ].join('') + + spyOn(EventHandler, 'trigger').and.callThrough() + const carouselEl = fixtureEl.querySelector('#myCarousel') + // eslint-disable-next-line no-new + new Carousel('#myCarousel') + expect(EventHandler.trigger).not.toHaveBeenCalledWith(carouselEl, 'hover.bs.carousel', jasmine.any(Function)) + }) + it('should go to previous item if left arrow key is pressed', () => { return new Promise(resolve => { fixtureEl.innerHTML = [ @@ -612,19 +660,21 @@ describe('Carousel', () => { }) }) - it('should call cycle on mouse out with pause equal to hover', () => { + it('should call `maybeCycle` on mouse out with pause equal to hover', () => { return new Promise(resolve => { - fixtureEl.innerHTML = '<div class="carousel"></div>' + fixtureEl.innerHTML = '<div class="carousel" data-bs-ride="true"></div>' const carouselEl = fixtureEl.querySelector('.carousel') const carousel = new Carousel(carouselEl) + spyOn(carousel, '_maybeEnableCycle').and.callThrough() spyOn(carousel, 'cycle') const mouseOutEvent = createEvent('mouseout') carouselEl.dispatchEvent(mouseOutEvent) setTimeout(() => { + expect(carousel._maybeEnableCycle).toHaveBeenCalled() expect(carousel.cycle).toHaveBeenCalled() resolve() }, 10) @@ -769,6 +819,28 @@ describe('Carousel', () => { expect(carousel._activeElement).toEqual(secondItemEl) }) + it('should continue cycling if it was already', () => { + fixtureEl.innerHTML = [ + '<div id="myCarousel" class="carousel slide">', + ' <div class="carousel-inner">', + ' <div class="carousel-item active">item 1</div>', + ' <div class="carousel-item">item 2</div>', + ' </div>', + '</div>' + ].join('') + + const carouselEl = fixtureEl.querySelector('#myCarousel') + const carousel = new Carousel(carouselEl) + spyOn(carousel, 'cycle') + + carousel.next() + expect(carousel.cycle).not.toHaveBeenCalled() + + carousel.cycle() + carousel.next() + expect(carousel.cycle).toHaveBeenCalled() + }) + it('should update indicators if present', () => { return new Promise(resolve => { fixtureEl.innerHTML = [ @@ -823,12 +895,14 @@ describe('Carousel', () => { const carousel = new Carousel(carouselEl) const nextSpy = spyOn(carousel, 'next') const prevSpy = spyOn(carousel, 'prev') + spyOn(carousel, '_maybeEnableCycle') nextBtnEl.click() prevBtnEl.click() expect(nextSpy).toHaveBeenCalled() expect(prevSpy).toHaveBeenCalled() + expect(carousel._maybeEnableCycle).toHaveBeenCalled() }) }) @@ -868,82 +942,32 @@ describe('Carousel', () => { }) describe('pause', () => { - it('should call cycle if the carousel have carousel-item-next or carousel-item-prev class, cause is sliding', () => { - fixtureEl.innerHTML = [ - '<div id="myCarousel" class="carousel slide">', - ' <div class="carousel-inner">', - ' <div class="carousel-item active">item 1</div>', - ' <div class="carousel-item carousel-item-next">item 2</div>', - ' <div class="carousel-item">item 3</div>', - ' </div>', - ' <div class="carousel-control-prev"></div>', - ' <div class="carousel-control-next"></div>', - '</div>' - ].join('') - - const carouselEl = fixtureEl.querySelector('#myCarousel') - const carousel = new Carousel(carouselEl) - - spyOn(carousel, 'cycle') - spyOn(carousel, '_clearInterval') - - carousel._slide('next') - carousel.pause() - - expect(carousel.cycle).toHaveBeenCalledWith(true) - expect(carousel._clearInterval).toHaveBeenCalled() - expect(carousel._stayPaused).toBeTrue() - }) - - it('should not call cycle if nothing is in transition', () => { - fixtureEl.innerHTML = [ - '<div id="myCarousel" class="carousel slide">', - ' <div class="carousel-inner">', - ' <div class="carousel-item active">item 1</div>', - ' <div class="carousel-item">item 2</div>', - ' <div class="carousel-item">item 3</div>', - ' </div>', - ' <div class="carousel-control-prev"></div>', - ' <div class="carousel-control-next"></div>', - '</div>' - ].join('') - - const carouselEl = fixtureEl.querySelector('#myCarousel') - const carousel = new Carousel(carouselEl) - - spyOn(carousel, 'cycle') - spyOn(carousel, '_clearInterval') - - carousel.pause() - - expect(carousel.cycle).not.toHaveBeenCalled() - expect(carousel._clearInterval).toHaveBeenCalled() - expect(carousel._stayPaused).toBeTrue() - }) - - it('should not set is paused at true if an event is passed', () => { - fixtureEl.innerHTML = [ - '<div id="myCarousel" class="carousel slide">', - ' <div class="carousel-inner">', - ' <div class="carousel-item active">item 1</div>', - ' <div class="carousel-item">item 2</div>', - ' <div class="carousel-item">item 3</div>', - ' </div>', - ' <div class="carousel-control-prev"></div>', - ' <div class="carousel-control-next"></div>', - '</div>' - ].join('') - - const carouselEl = fixtureEl.querySelector('#myCarousel') - const carousel = new Carousel(carouselEl) - const event = createEvent('mouseenter') + it('should trigger transitionend if the carousel have carousel-item-next or carousel-item-prev class, cause is sliding', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = [ + '<div id="myCarousel" class="carousel slide">', + ' <div class="carousel-inner">', + ' <div class="carousel-item active">item 1</div>', + ' <div class="carousel-item carousel-item-next">item 2</div>', + ' <div class="carousel-item">item 3</div>', + ' </div>', + ' <div class="carousel-control-prev"></div>', + ' <div class="carousel-control-next"></div>', + '</div>' + ].join('') - spyOn(carousel, '_clearInterval') + const carouselEl = fixtureEl.querySelector('#myCarousel') + const carousel = new Carousel(carouselEl) - carousel.pause(event) + carouselEl.addEventListener('transitionend', () => { + expect(carousel._clearInterval).toHaveBeenCalled() + resolve() + }) - expect(carousel._clearInterval).toHaveBeenCalled() - expect(carousel._stayPaused).toBeFalse() + spyOn(carousel, '_clearInterval') + carousel._slide('next') + carousel.pause() + }) }) }) @@ -971,30 +995,6 @@ describe('Carousel', () => { expect(window.setInterval).toHaveBeenCalled() }) - it('should not set interval if the carousel is paused', () => { - fixtureEl.innerHTML = [ - '<div id="myCarousel" class="carousel slide">', - ' <div class="carousel-inner">', - ' <div class="carousel-item active">item 1</div>', - ' <div class="carousel-item">item 2</div>', - ' <div class="carousel-item">item 3</div>', - ' </div>', - ' <div class="carousel-control-prev"></div>', - ' <div class="carousel-control-next"></div>', - '</div>' - ].join('') - - const carouselEl = fixtureEl.querySelector('#myCarousel') - const carousel = new Carousel(carouselEl) - - spyOn(window, 'setInterval').and.callThrough() - - carousel._stayPaused = true - carousel.cycle(true) - - expect(window.setInterval).not.toHaveBeenCalled() - }) - it('should clear interval if there is one', () => { fixtureEl.innerHTML = [ '<div id="myCarousel" class="carousel slide">', @@ -1132,7 +1132,7 @@ describe('Carousel', () => { expect(spy).not.toHaveBeenCalled() }) - it('should call pause and cycle is the provided is the same compare to the current one', () => { + it('should not continue if the provided is the same compare to the current one', () => { fixtureEl.innerHTML = [ '<div id="myCarousel" class="carousel slide">', ' <div class="carousel-inner">', @@ -1153,8 +1153,6 @@ describe('Carousel', () => { carousel.to(0) expect(carousel._slide).not.toHaveBeenCalled() - expect(carousel.pause).toHaveBeenCalled() - expect(carousel.cycle).toHaveBeenCalled() }) it('should wait before performing to if a slide is sliding', () => { @@ -1449,8 +1447,8 @@ describe('Carousel', () => { const loadEvent = createEvent('load') window.dispatchEvent(loadEvent) - - expect(Carousel.getInstance(carouselEl)).not.toBeNull() + const carousel = Carousel.getInstance(carouselEl) + expect(carousel._interval).not.toBeNull() }) it('should create carousel and go to the next slide on click (with real button controls)', () => { @@ -1508,7 +1506,7 @@ describe('Carousel', () => { it('should create carousel and go to the next slide on click with data-bs-slide-to', () => { return new Promise(resolve => { fixtureEl.innerHTML = [ - '<div id="myCarousel" class="carousel slide">', + '<div id="myCarousel" class="carousel slide" data-bs-ride="true">', ' <div class="carousel-inner">', ' <div class="carousel-item active">item 1</div>', ' <div id="item2" class="carousel-item">item 2</div>', @@ -1525,6 +1523,7 @@ describe('Carousel', () => { setTimeout(() => { expect(item2).toHaveClass('active') + expect(Carousel.getInstance('#myCarousel')._interval).not.toBeNull() resolve() }, 10) }) diff --git a/site/content/docs/5.1/components/carousel.md b/site/content/docs/5.1/components/carousel.md index 14f91911d2..782d620aaa 100644 --- a/site/content/docs/5.1/components/carousel.md +++ b/site/content/docs/5.1/components/carousel.md @@ -77,7 +77,7 @@ Adding in the previous and next controls. We recommend using `<button>` elements You can also add the indicators to the carousel, alongside the controls, too. {{< example >}} -<div id="carouselExampleIndicators" class="carousel slide" data-bs-ride="carousel"> +<div id="carouselExampleIndicators" class="carousel slide" data-bs-ride="true"> <div class="carousel-indicators"> <button type="button" data-bs-target="#carouselExampleIndicators" data-bs-slide-to="0" class="active" aria-current="true" aria-label="Slide 1"></button> <button type="button" data-bs-target="#carouselExampleIndicators" data-bs-slide-to="1" aria-label="Slide 2"></button> @@ -110,7 +110,7 @@ You can also add the indicators to the carousel, alongside the controls, too. Add captions to your slides easily with the `.carousel-caption` element within any `.carousel-item`. They can be easily hidden on smaller viewports, as shown below, with optional [display utilities]({{< docsref "/utilities/display" >}}). We hide them initially with `.d-none` and bring them back on medium-sized devices with `.d-md-block`. {{< example >}} -<div id="carouselExampleCaptions" class="carousel slide" data-bs-ride="carousel"> +<div id="carouselExampleCaptions" class="carousel slide" data-bs-ride="false"> <div class="carousel-indicators"> <button type="button" data-bs-target="#carouselExampleCaptions" data-bs-slide-to="0" class="active" aria-current="true" aria-label="Slide 1"></button> <button type="button" data-bs-target="#carouselExampleCaptions" data-bs-slide-to="1" aria-label="Slide 2"></button> @@ -318,9 +318,9 @@ var carousel = new bootstrap.Carousel(myCarousel) | `interval` | number | `5000` | The amount of time to delay between automatically cycling an item. If `false`, carousel will not automatically cycle. | | `keyboard` | boolean | `true` | Whether the carousel should react to keyboard events. | | `pause` | string, boolean | `"hover"` | If set to `"hover"`, pauses the cycling of the carousel on `mouseenter` and resumes the cycling of the carousel on `mouseleave`. If set to `false`, hovering over the carousel won't pause it. On touch-enabled devices, when set to `"hover"`, cycling will pause on `touchend` (once the user finished interacting with the carousel) for two intervals, before automatically resuming. This is in addition to the mouse behavior. | -| `ride` | string, boolean | `false` | Autoplays the carousel after the user manually cycles the first item. If set to`"carousel"`, autoplays the carousel on load. | -| `wrap` | boolean | `true` | Whether the carousel should cycle continuously or have hard stops. | +| `ride` | string, boolean | `false` | If set to `true`, autoplays the carousel after the user manually cycles the first item. If set to `"carousel"`, autoplays the carousel on load. | | `touch` | boolean | `true` | Whether the carousel should support left/right swipe interactions on touchscreen devices. | +| `wrap` | boolean | `true` | Whether the carousel should cycle continuously or have hard stops. | {{< /bs-table >}} ### Methods |