amp-carousel-0.2: fix keyboard navigation. (#37178)

* amp-carousel-0.2: fix keyboard navigation. * address kbax nits.
ampproject · Dec 13, 2021 · d8b28f4 · d8b28f4
1 parent d269c20
commit d8b28f4
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 9 deletions.
diff --git a/extensions/amp-carousel/0.1/carousel-controls.js b/extensions/amp-carousel/0.1/carousel-controls.js
@@ -72,15 +72,17 @@ export class CarouselControls {
    * @param {*} onInteraction
    */
   setupButtonInteraction(button, onInteraction) {
-    button.onkeydown = (event) => {
+    button.addEventListener('click', onInteraction);
+    button.addEventListener('keydown', (event) => {
+      if (event.defaultPrevented) {
+        return;
+      }
+
       if (event.key == Keys_Enum.ENTER || event.key == Keys_Enum.SPACE) {
-        if (!event.defaultPrevented) {
-          event.preventDefault();
-          onInteraction();
-        }
+        event.preventDefault();
+        onInteraction();
       }
-    };
-    button.onclick = onInteraction;
+    });
   }
 
   /**

diff --git a/extensions/amp-carousel/0.2/amp-carousel.js b/extensions/amp-carousel/0.2/amp-carousel.js
@@ -1,3 +1,4 @@
+import {Keys_Enum} from '#core/constants/key-codes';
 import {ActionSource} from '../../amp-base-carousel/0.1/action-source';
 import {ActionTrust_Enum} from '#core/constants/action-constants';
 import {CSS} from '../../../build/amp-carousel-0.2.css';
@@ -163,8 +164,8 @@ class AmpCarousel extends AMP.BaseElement {
         this.onScrollPositionChanged_();
       }
     );
-    this.prevButton_.addEventListener('click', () => this.interactionPrev());
-    this.nextButton_.addEventListener('click', () => this.interactionNext());
+    this.setupButtonInteraction(this.nextButton_, () => this.interactionNext());
+    this.setupButtonInteraction(this.prevButton_, () => this.interactionPrev());
     this.handlePropagationInViewer_();
 
     const owners = Services.ownersForDoc(element);
@@ -208,6 +209,24 @@ class AmpCarousel extends AMP.BaseElement {
     return this.mutateElement(() => {});
   }
 
+  /**
+   * @param {!HTMLDivElement} button
+   * @param {*} onInteraction
+   */
+  setupButtonInteraction(button, onInteraction) {
+    button.addEventListener('click', onInteraction);
+    button.addEventListener('keydown', (event) => {
+      if (event.defaultPrevented) {
+        return;
+      }
+
+      if (event.key == Keys_Enum.ENTER || event.key == Keys_Enum.SPACE) {
+        event.preventDefault();
+        onInteraction();
+      }
+    });
+  }
+
   /** @override */
   isRelayoutNeeded() {
     return true;

diff --git a/extensions/amp-carousel/0.2/test/test-type-slides.js b/extensions/amp-carousel/0.2/test/test-type-slides.js
@@ -214,6 +214,24 @@ describes.realWin(
         expect(slideWrappers[2].getAttribute('aria-hidden')).to.equal('true');
       });
 
+      it('should go to the correct slide when navigating with keyboard', async () => {
+        const carousel = await getCarousel({loop: true});
+        const slideWrappers = getSlideWrappers(carousel);
+        const kbEnterEvent = new KeyboardEvent('keydown', {'key': 'Enter'});
+
+        getNextButton(carousel).dispatchEvent(kbEnterEvent);
+        await afterIndexUpdate(carousel);
+        expect(slideWrappers[0].getAttribute('aria-hidden')).to.equal('true');
+        expect(slideWrappers[1].getAttribute('aria-hidden')).to.equal('false');
+        expect(slideWrappers[2].getAttribute('aria-hidden')).to.equal('true');
+
+        getPrevButton(carousel).dispatchEvent(kbEnterEvent);
+        await afterIndexUpdate(carousel);
+        expect(slideWrappers[0].getAttribute('aria-hidden')).to.equal('false');
+        expect(slideWrappers[1].getAttribute('aria-hidden')).to.equal('true');
+        expect(slideWrappers[2].getAttribute('aria-hidden')).to.equal('true');
+      });
+
       it('should go to the correct slide clicking prev', async () => {
         const carousel = await getCarousel({loop: true});
         const slideWrappers = getSlideWrappers(carousel);