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

diff --git a/js/src/carousel.js b/js/src/carousel.js
index 7a30beb10e3d9355845668cd59a8e2cf5e514f1b..64f38d7e64d45916767a168fee16bb1e62bb31af 100644 (file)
--- 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 8875f3f00382c5a68d2f77bcc6eb308099228806..feaa3f5fe08b9bbf7f522cf222564e682282df62 100644 (file)
--- 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 14f91911d28cccd5659bebf97bd01851ae76e377..782d620aaa74bc05d85dff2eb09660dfeb9c8ae8 100644 (file)
--- 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