fix(menu): added focus trapping, improved compatibility with screen r…

…eaders (#24076) * fix(menu): add basic accessibility features * fix(menu): add focus trapping * test(menu): add test for focus trapping * style(menu): lint fixes * fix(menu): focus first element inside instead of whole menu * test(menu): fix focus trap test to account for new behavior * refactor(menu): pull focus handler into its own prop * test(menu): add a11y testing * fix(menu): prevent nested aria landmark from header inside menu * fix(menu): revert switch to nav element * fix(menu): remove unnecessary import from test * fix(menu): allow for custom aria-label * fix(menu): move nested ARIA role logic to header for flexibility * fix(item): only add focusable class if it actually is focusable * fix(menu): allow focusing of menu itself, for a11y on menus with no focusable children * fix(item): move isFocusable logic to state for better reactivity * perf(item): only grab one focusable child * fix(menu): hide page content from screen readers when menu is open * fix(menu): fallback to focusing host element * docs(menu): add comments Co-authored-by: Liam DeBeasi <[email protected]>
ionic-team · Oct 27, 2021 · bdb268a · bdb268a
1 parent 2916810
commit bdb268a
Show file tree

Hide file tree

Showing 7 changed files with 224 additions and 9 deletions.
diff --git a/core/src/components/header/header.tsx b/core/src/components/header/header.tsx
@@ -2,6 +2,7 @@ import { Component, ComponentInterface, Element, Host, Prop, h, writeTask } from
 
 import { getIonMode } from '../../global/ionic-global';
 import { inheritAttributes } from '../../utils/helpers';
+import { hostContext } from '../../utils/theme';
 
 import { cloneElement, createHeaderIndex, handleContentScroll, handleToolbarIntersection, setHeaderActive, setToolbarBackgroundOpacity } from './header.utils';
 
@@ -154,9 +155,12 @@ export class Header implements ComponentInterface {
     const mode = getIonMode(this);
     const collapse = this.collapse || 'none';
 
+    // banner role must be at top level, so remove role if inside a menu
+    const roleType = hostContext('ion-menu', this.el) ? 'none' : 'banner';
+
     return (
       <Host
-        role="banner"
+        role={roleType}
         class={{
           [mode]: true,
 

diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx
@@ -35,6 +35,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
   @Element() el!: HTMLIonItemElement;
 
   @State() multipleInputs = false;
+  @State() focusable = true;
 
   /**
    * The color to use from your application's color palette.
@@ -173,7 +174,10 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
   }
 
   componentDidLoad() {
-    raf(() => this.setMultipleInputs());
+    raf(() => {
+      this.setMultipleInputs();
+      this.focusable = this.isFocusable();
+    });
   }
 
   // If the item contains multiple clickable elements and/or inputs, then the item
@@ -217,6 +221,11 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
     return (this.isClickable() || this.hasCover());
   }
 
+  private isFocusable(): boolean {
+    const focusableChild = this.el.querySelector('.ion-focusable');
+    return (this.canActivate() || focusableChild !== null);
+  }
+
   private getFirstInput(): HTMLIonInputElement | HTMLIonTextareaElement {
     const inputs = this.el.querySelectorAll('ion-input, ion-textarea') as NodeListOf<HTMLIonInputElement | HTMLIonTextareaElement>;
     return inputs[0];
@@ -289,7 +298,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
             'in-list': hostContext('ion-list', this.el),
             'item-multiple-inputs': this.multipleInputs,
             'ion-activatable': canActivate,
-            'ion-focusable': true,
+            'ion-focusable': this.focusable
           })
         }}
       >

diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx
@@ -5,13 +5,14 @@ import { getIonMode } from '../../global/ionic-global';
 import { Animation, Gesture, GestureDetail, MenuChangeEventDetail, MenuI, Side } from '../../interface';
 import { getTimeGivenProgression } from '../../utils/animation/cubic-bezier';
 import { GESTURE_CONTROLLER } from '../../utils/gesture';
-import { assert, clamp, isEndSide as isEnd } from '../../utils/helpers';
+import { assert, clamp, inheritAttributes, isEndSide as isEnd } from '../../utils/helpers';
 import { menuController } from '../../utils/menu-controller';
 
 const iosEasing = 'cubic-bezier(0.32,0.72,0,1)';
 const mdEasing = 'cubic-bezier(0.0,0.0,0.2,1)';
 const iosEasingReverse = 'cubic-bezier(1, 0, 0.68, 0.28)';
 const mdEasingReverse = 'cubic-bezier(0.4, 0, 0.6, 1)';
+const focusableQueryString = '[tabindex]:not([tabindex^="-"]), input:not([type=hidden]):not([tabindex^="-"]), textarea:not([tabindex^="-"]), button:not([tabindex^="-"]), select:not([tabindex^="-"]), .ion-focusable:not([tabindex^="-"])';
 
 /**
  * @part container - The container for the menu content.
@@ -39,6 +40,11 @@ export class Menu implements ComponentInterface, MenuI {
   backdropEl?: HTMLElement;
   menuInnerEl?: HTMLElement;
   contentEl?: HTMLElement;
+  lastFocus?: HTMLElement;
+
+  private inheritedAttributes: { [k: string]: any } = {};
+
+  private handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document);
 
   @Element() el!: HTMLIonMenuElement;
 
@@ -159,6 +165,7 @@ export class Menu implements ComponentInterface, MenuI {
 
     const el = this.el;
     const parent = el.parentNode as any;
+
     if (this.contentId === undefined) {
       console.warn(`[DEPRECATED][ion-menu] Using the [main] attribute is deprecated, please use the "contentId" property instead:
 BEFORE:
@@ -205,6 +212,10 @@ AFTER:
     this.updateState();
   }
 
+  componentWillLoad() {
+    this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
+  }
+
   async componentDidLoad() {
     this.ionMenuChange.emit({ disabled: this.disabled, open: this._isOpen });
     this.updateState();
@@ -246,6 +257,13 @@ AFTER:
     }
   }
 
+  @Listen('keydown')
+  onKeydown(ev: KeyboardEvent) {
+    if (ev.key === 'Escape') {
+      this.close();
+    }
+  }
+
   /**
    * Returns `true` is the menu is open.
    */
@@ -301,6 +319,65 @@ AFTER:
     return menuController._setOpen(this, shouldOpen, animated);
   }
 
+  private focusFirstDescendant() {
+    const { el } = this;
+    const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null;
+
+    if (firstInput) {
+      firstInput.focus();
+    } else {
+      el.focus();
+    }
+  }
+
+  private focusLastDescendant() {
+    const { el } = this;
+    const inputs = Array.from(el.querySelectorAll<HTMLElement>(focusableQueryString));
+    const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null;
+
+    if (lastInput) {
+      lastInput.focus();
+    } else {
+      el.focus();
+    }
+  }
+
+  private trapKeyboardFocus(ev: Event, doc: Document) {
+    const target = ev.target as HTMLElement | null;
+    if (!target) { return; }
+
+    /**
+     * If the target is inside the menu contents, let the browser
+     * focus as normal and keep a log of the last focused element.
+     */
+    if (this.el.contains(target)) {
+      this.lastFocus = target;
+    } else {
+      /**
+       * Otherwise, we are about to have focus go out of the menu.
+       * Wrap the focus to either the first or last element.
+       */
+
+      /**
+       * Once we call `focusFirstDescendant`, another focus event
+       * will fire, which will cause `lastFocus` to be updated
+       * before we can run the code after that. We cache the value
+       * here to avoid that.
+       */
+      this.focusFirstDescendant();
+
+      /**
+       * If the cached last focused element is the same as the now-
+       * active element, that means the user was on the first element
+       * already and pressed Shift + Tab, so we need to wrap to the
+       * last descendant.
+       */
+      if (this.lastFocus === doc.activeElement) {
+        this.focusLastDescendant();
+      }
+    }
+  }
+
   async _setOpen(shouldOpen: boolean, animated = true): Promise<boolean> {
     // If the menu is disabled or it is currently being animated, let's do nothing
     if (!this._isActive() || this.isAnimating || shouldOpen === this._isOpen) {
@@ -479,6 +556,16 @@ AFTER:
     // this places the menu into the correct location before it animates in
     // this css class doesn't actually kick off any animations
     this.el.classList.add(SHOW_MENU);
+
+    /**
+     * We add a tabindex here so that focus trapping
+     * still works even if the menu does not have
+     * any focusable elements slotted inside. The
+     * focus trapping utility will fallback to focusing
+     * the menu so focus does not leave when the menu
+     * is open.
+     */
+    this.el.setAttribute('tabindex', '0');
     if (this.backdropEl) {
       this.backdropEl.classList.add(SHOW_BACKDROP);
     }
@@ -505,19 +592,51 @@ AFTER:
     }
 
     if (isOpen) {
-      // add css class
+      // add css class and hide content behind menu from screen readers
       if (this.contentEl) {
         this.contentEl.classList.add(MENU_CONTENT_OPEN);
+
+        /**
+         * When the menu is open and overlaying the main
+         * content, the main content should not be announced
+         * by the screenreader as the menu is the main
+         * focus. This is useful with screenreaders that have
+         * "read from top" gestures that read the entire
+         * page from top to bottom when activated.
+         */
+        this.contentEl.setAttribute('aria-hidden', 'true');
       }
 
       // emit open event
       this.ionDidOpen.emit();
+
+      // focus menu content for screen readers
+      if (this.menuInnerEl) {
+        this.focusFirstDescendant();
+      }
+
+      // setup focus trapping
+      document.addEventListener('focus', this.handleFocus, true);
     } else {
-      // remove css classes
+      // remove css classes and unhide content from screen readers
       this.el.classList.remove(SHOW_MENU);
+
+      /**
+       * Remove tabindex from the menu component
+       * so that is cannot be tabbed to.
+       */
+      this.el.removeAttribute('tabindex');
       if (this.contentEl) {
         this.contentEl.classList.remove(MENU_CONTENT_OPEN);
+
+        /**
+         * Remove aria-hidden so screen readers
+         * can announce the main content again
+         * now that the menu is not the main focus.
+         */
+        this.contentEl.removeAttribute('aria-hidden');
       }
+
       if (this.backdropEl) {
         this.backdropEl.classList.remove(SHOW_BACKDROP);
       }
@@ -528,6 +647,9 @@ AFTER:
 
       // emit close event
       this.ionDidClose.emit();
+
+      // undo focus trapping so multiple menus don't collide
+      document.removeEventListener('focus', this.handleFocus, true);
     }
   }
 
@@ -561,12 +683,13 @@ AFTER:
   }
 
   render() {
-    const { isEndSide, type, disabled, isPaneVisible } = this;
+    const { isEndSide, type, disabled, isPaneVisible, inheritedAttributes } = this;
     const mode = getIonMode(this);
 
     return (
       <Host
         role="navigation"
+        aria-label={inheritedAttributes['aria-label'] || 'menu'}
         class={{
           [mode]: true,
           [`menu-type-${type}`]: true,

diff --git a/core/src/components/menu/test/a11y/e2e.ts b/core/src/components/menu/test/a11y/e2e.ts
@@ -0,0 +1,15 @@
+import { newE2EPage } from '@stencil/core/testing';
+import { AxePuppeteer } from '@axe-core/puppeteer';
+
+test('menu: axe', async () => {
+  const page = await newE2EPage({
+    url: '/src/components/menu/test/a11y?ionic:_testing=true'
+  });
+
+  const menu = await page.find('ion-menu');
+  await menu.callMethod('open');
+  await menu.waitForVisible();
+
+  const results = await new AxePuppeteer(page).analyze();
+  expect(results.violations.length).toEqual(0);
+});
diff --git a/core/src/components/menu/test/a11y/index.html b/core/src/components/menu/test/a11y/index.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+
+<head>
+  <meta charset="UTF-8">
+  <title>Segment - a11y</title>
+  <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
+  <link href="../../../../../css/core.css" rel="stylesheet">
+  <link href="../../../../../scripts/testing/styles.css" rel="stylesheet">
+  <script src="../../../../../scripts/testing/scripts.js"></script>
+  <script nomodule src="../../../../../dist/ionic/ionic.js"></script>
+  <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+</head>
+
+<body>
+  <main>
+    <h1>Menu</h1>
+    <ion-menu menu-id="menu" content-id="main-content">
+      <ion-header>
+        <ion-toolbar>
+          <ion-title>Menu</ion-title>
+        </ion-toolbar>
+      </ion-header>
+      <ion-content>
+        <ion-list>
+          <ion-item>
+            <ion-button>Button</ion-button>
+          </ion-item>
+          <ion-item>
+            <ion-button>Button 2</ion-button>
+          </ion-item>
+          <ion-item>Menu Item</ion-item>
+          <ion-item>Menu Item</ion-item>
+          <ion-item>Menu Item</ion-item>
+        </ion-list>
+      </ion-content>
+    </ion-menu>
+  </main>
+</body>
+
+</html>
diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts
@@ -1,6 +1,11 @@
 import { testMenu } from '../test.utils';
+import { newE2EPage } from '@stencil/core/testing';
 
 const DIRECTORY = 'basic';
+const getActiveElementID = async (page) => {
+  const activeElement = await page.evaluateHandle(() => document.activeElement);
+  return await page.evaluate(el => el && el.id, activeElement);
+}
 
 test('menu: start menu', async () => {
   await testMenu(DIRECTORY, '#start-menu', 'first');
@@ -14,6 +19,21 @@ test('menu: end menu', async () => {
   await testMenu(DIRECTORY, '#end-menu');
 });
 
+test('menu: focus trap', async () => {
+  const page = await newE2EPage({ url: '/src/components/menu/test/basic?ionic:_testing=true' });
+
+  await page.click('#open-first');
+  const menu = await page.find('#start-menu');
+  await menu.waitForVisible();
+
+  let activeElID = await getActiveElementID(page);
+  expect(activeElID).toEqual('start-menu-button');
+
+  await page.keyboard.press('Tab');
+  activeElID = await getActiveElementID(page);
+  expect(activeElID).toEqual('start-menu-button');
+});
+
 /**
  * RTL Tests
  */