gh-407: only allow keyboard navigation if focus is within carousel

leandrowd · Apr 5, 2020 · 56b2af5 · 56b2af5
1 parent 0f78cfe
commit 56b2af5
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 1 deletion.
diff --git a/src/__tests__/Carousel.js b/src/__tests__/Carousel.js
@@ -271,28 +271,40 @@ describe('Slider', function() {
                 componentInstance.decrement = jest.genMockFunction();
             });
 
+            it('should not navigate if the focus is outside of the carousel', () => {
+                componentInstance.navigateWithKeyboard({ keyCode: 39 });
+                componentInstance.navigateWithKeyboard({ keyCode: 37 });
+
+                expect(componentInstance.increment.mock.calls.length).toBe(0);
+                expect(componentInstance.decrement.mock.calls.length).toBe(0);
+            });
+
             it('should call only increment on ArrowRight (39)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 39 });
 
                 expect(componentInstance.increment.mock.calls.length).toBe(1);
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);
             });
 
             it('should call only decrement on ArrowLeft (37)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 37 });
 
                 expect(componentInstance.decrement.mock.calls.length).toBe(1);
                 expect(componentInstance.increment.mock.calls.length).toBe(0);
             });
 
             it('should not call increment on ArrowDown (40)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 40 });
 
                 expect(componentInstance.increment.mock.calls.length).toBe(0);
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);
             });
 
             it('should not call decrement on ArrowUp (38)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 38 });
 
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);
@@ -311,28 +323,40 @@ describe('Slider', function() {
                 componentInstance.decrement = jest.genMockFunction();
             });
 
+            it('should not navigate if the focus is outside of the carousel', () => {
+                componentInstance.navigateWithKeyboard({ keyCode: 40 });
+                componentInstance.navigateWithKeyboard({ keyCode: 38 });
+
+                expect(componentInstance.increment.mock.calls.length).toBe(0);
+                expect(componentInstance.decrement.mock.calls.length).toBe(0);
+            });
+
             it('should call only increment on ArrowDown (40)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 40 });
 
                 expect(componentInstance.increment.mock.calls.length).toBe(1);
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);
             });
 
             it('should call only decrement on ArrowUp (38)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 38 });
 
                 expect(componentInstance.decrement.mock.calls.length).toBe(1);
                 expect(componentInstance.increment.mock.calls.length).toBe(0);
             });
 
             it('should not call increment on ArrowRight (39)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 39 });
 
                 expect(componentInstance.increment.mock.calls.length).toBe(0);
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);
             });
 
             it('should not call decrement on ArrowLeft (37)', () => {
+                componentInstance.carouselWrapperRef.focus();
                 componentInstance.navigateWithKeyboard({ keyCode: 37 });
 
                 expect(componentInstance.decrement.mock.calls.length).toBe(0);

diff --git a/src/__tests__/__snapshots__/Carousel.js.snap b/src/__tests__/__snapshots__/Carousel.js.snap
@@ -3,6 +3,7 @@
 exports[`Slider Snapshots center mode 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -350,6 +351,7 @@ exports[`Slider Snapshots center mode 1`] = `
 exports[`Slider Snapshots custom class name 1`] = `
 <div
   className="my-custom-carousel"
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -662,6 +664,7 @@ exports[`Slider Snapshots custom class name 1`] = `
 exports[`Slider Snapshots custom width 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -974,6 +977,7 @@ exports[`Slider Snapshots custom width 1`] = `
 exports[`Slider Snapshots default 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -1286,6 +1290,7 @@ exports[`Slider Snapshots default 1`] = `
 exports[`Slider Snapshots infinite loop 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -1614,6 +1619,7 @@ exports[`Slider Snapshots infinite loop 1`] = `
 exports[`Slider Snapshots no arrows 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -1928,6 +1934,7 @@ exports[`Slider Snapshots no children at mount 1`] = `null`;
 exports[`Slider Snapshots no indicators 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -2173,6 +2180,7 @@ exports[`Slider Snapshots no indicators 1`] = `
 exports[`Slider Snapshots no indicators 2`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -2480,6 +2488,7 @@ exports[`Slider Snapshots no indicators 2`] = `
 exports[`Slider Snapshots no thumbs 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -2664,6 +2673,7 @@ exports[`Slider Snapshots no thumbs 1`] = `
 exports[`Slider Snapshots swipeable false 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"
@@ -2973,6 +2983,7 @@ exports[`Slider Snapshots swipeable false 1`] = `
 exports[`Slider Snapshots vertical axis 1`] = `
 <div
   className={undefined}
+  tabIndex="0"
 >
   <div
     className="carousel carousel-slider"

diff --git a/src/components/Carousel.js b/src/components/Carousel.js
@@ -268,7 +268,22 @@ class Carousel extends Component {
         this.autoPlay();
     };
 
+    isFocusWithinTheCarousel = () => {
+        if (
+            document.activeElement === this.carouselWrapperRef ||
+            this.carouselWrapperRef.contains(document.activeElement)
+        ) {
+            return true;
+        }
+
+        return false;
+    };
+
     navigateWithKeyboard = (e) => {
+        if (!this.isFocusWithinTheCarousel()) {
+            return;
+        }
+
         const { axis } = this.props;
         const isHorizontal = axis === 'horizontal';
         const keyNames = {
@@ -731,7 +746,7 @@ class Carousel extends Component {
             containerStyles.height = this.state.itemSize;
         }
         return (
-            <div className={this.props.className} ref={this.setCarouselWrapperRef}>
+            <div className={this.props.className} ref={this.setCarouselWrapperRef} tabIndex="0">
                 <div className={klass.CAROUSEL(true)} style={{ width: this.props.width }}>
                     <button
                         type="button"