fix(carousel): arrow keys break animation if carousel sliding (#34307)

twbs · Jun 22, 2021 · 290b9ee · 290b9ee
1 parent 8be957b
commit 290b9ee
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 15 deletions.
diff --git a/js/src/carousel.js b/js/src/carousel.js
@@ -59,6 +59,11 @@ const ORDER_PREV = 'prev'
 const DIRECTION_LEFT = 'left'
 const DIRECTION_RIGHT = 'right'
 
+const KEY_TO_DIRECTION = {
+  [ARROW_LEFT_KEY]: DIRECTION_RIGHT,
+  [ARROW_RIGHT_KEY]: DIRECTION_LEFT
+}
+
 const EVENT_SLIDE = `slide${EVENT_KEY}`
 const EVENT_SLID = `slid${EVENT_KEY}`
 const EVENT_KEYDOWN = `keydown${EVENT_KEY}`
@@ -134,9 +139,7 @@ class Carousel extends BaseComponent {
   // Public
 
   next() {
-    if (!this._isSliding) {
-      this._slide(ORDER_NEXT)
-    }
+    this._slide(ORDER_NEXT)
   }
 
   nextWhenVisible() {
@@ -148,9 +151,7 @@ class Carousel extends BaseComponent {
   }
 
   prev() {
-    if (!this._isSliding) {
-      this._slide(ORDER_PREV)
-    }
+    this._slide(ORDER_PREV)
   }
 
   pause(event) {
@@ -319,12 +320,10 @@ class Carousel extends BaseComponent {
       return
     }
 
-    if (event.key === ARROW_LEFT_KEY) {
-      event.preventDefault()
-      this._slide(DIRECTION_RIGHT)
-    } else if (event.key === ARROW_RIGHT_KEY) {
+    const direction = KEY_TO_DIRECTION[event.key]
+    if (direction) {
       event.preventDefault()
-      this._slide(DIRECTION_LEFT)
+      this._slide(direction)
     }
   }
 
@@ -408,6 +407,10 @@ class Carousel extends BaseComponent {
       return
     }
 
+    if (this._isSliding) {
+      return
+    }
+
     const slideEvent = this._triggerSlideEvent(nextElement, eventDirectionName)
     if (slideEvent.defaultPrevented) {
       return

diff --git a/js/tests/unit/carousel.spec.js b/js/tests/unit/carousel.spec.js
@@ -203,6 +203,26 @@ describe('Carousel', () => {
       expect(spySlide).not.toHaveBeenCalled()
     })
 
+    it('should not slide if arrow key is pressed and carousel is sliding', () => {
+      fixtureEl.innerHTML = '<div></div>'
+
+      const carouselEl = fixtureEl.querySelector('div')
+      const carousel = new Carousel(carouselEl, {})
+
+      spyOn(carousel, '_triggerSlideEvent')
+
+      carousel._isSliding = true;
+
+      ['ArrowLeft', 'ArrowRight'].forEach(key => {
+        const keydown = createEvent('keydown')
+        keydown.key = key
+
+        carouselEl.dispatchEvent(keydown)
+      })
+
+      expect(carousel._triggerSlideEvent).not.toHaveBeenCalled()
+    })
+
     it('should wrap around from end to start when wrap option is true', done => {
       fixtureEl.innerHTML = [
         '<div id="myCarousel" class="carousel slide">',
@@ -487,6 +507,49 @@ describe('Carousel', () => {
       })
     })
 
+    it('should not slide when swiping and carousel is sliding', done => {
+      Simulator.setType('touch')
+      clearPointerEvents()
+      document.documentElement.ontouchstart = () => {}
+
+      fixtureEl.innerHTML = [
+        '<div class="carousel" data-bs-interval="false">',
+        '  <div class="carousel-inner">',
+        '    <div id="item" class="carousel-item active">',
+        '      <img alt="">',
+        '    </div>',
+        '    <div class="carousel-item">',
+        '      <img alt="">',
+        '    </div>',
+        '  </div>',
+        '</div>'
+      ].join('')
+
+      const carouselEl = fixtureEl.querySelector('.carousel')
+      const carousel = new Carousel(carouselEl)
+      carousel._isSliding = true
+
+      spyOn(carousel, '_triggerSlideEvent')
+
+      Simulator.gestures.swipe(carouselEl, {
+        deltaX: 300,
+        deltaY: 0
+      })
+
+      Simulator.gestures.swipe(carouselEl, {
+        pos: [300, 10],
+        deltaX: -300,
+        deltaY: 0
+      })
+
+      setTimeout(() => {
+        expect(carousel._triggerSlideEvent).not.toHaveBeenCalled()
+        delete document.documentElement.ontouchstart
+        restorePointerEvents()
+        done()
+      }, 300)
+    })
+
     it('should not allow pinch with touch events', done => {
       Simulator.setType('touch')
       clearPointerEvents()
@@ -552,12 +615,12 @@ describe('Carousel', () => {
       const carouselEl = fixtureEl.querySelector('div')
       const carousel = new Carousel(carouselEl, {})
 
-      spyOn(carousel, '_slide')
+      spyOn(carousel, '_triggerSlideEvent')
 
       carousel._isSliding = true
       carousel.next()
 
-      expect(carousel._slide).not.toHaveBeenCalled()
+      expect(carousel._triggerSlideEvent).not.toHaveBeenCalled()
     })
 
     it('should not fire slid when slide is prevented', done => {
@@ -763,12 +826,12 @@ describe('Carousel', () => {
       const carouselEl = fixtureEl.querySelector('div')
       const carousel = new Carousel(carouselEl, {})
 
-      spyOn(carousel, '_slide')
+      spyOn(carousel, '_triggerSlideEvent')
 
       carousel._isSliding = true
       carousel.prev()
 
-      expect(carousel._slide).not.toHaveBeenCalled()
+      expect(carousel._triggerSlideEvent).not.toHaveBeenCalled()
     })
   })