From bdb268aa12c5bf411c96529672486d35e018cefa Mon Sep 17 00:00:00 2001 From: Amanda Smith <90629384+amandaesmith3@users.noreply.github.com> Date: Wed, 27 Oct 2021 07:51:46 -0500 Subject: [PATCH] fix(menu): added focus trapping, improved compatibility with screen readers (#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 --- core/src/components/header/header.tsx | 6 +- core/src/components/item/item.tsx | 13 +- core/src/components/menu/menu.tsx | 131 +++++++++++++++++- core/src/components/menu/test/a11y/e2e.ts | 15 ++ core/src/components/menu/test/a11y/index.html | 41 ++++++ core/src/components/menu/test/basic/e2e.ts | 20 +++ .../src/components/menu/test/basic/index.html | 7 +- 7 files changed, 224 insertions(+), 9 deletions(-) create mode 100644 core/src/components/menu/test/a11y/e2e.ts create mode 100644 core/src/components/menu/test/a11y/index.html diff --git a/core/src/components/header/header.tsx b/core/src/components/header/header.tsx index 825c690ad3..c538a7b5b5 100644 --- 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 ( 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; 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 index 452125a29a..fe689a66c7 100644 --- 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(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 { // 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 ( { + 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); +}); \ No newline at end of file diff --git a/core/src/components/menu/test/a11y/index.html b/core/src/components/menu/test/a11y/index.html new file mode 100644 index 0000000000..036277bc13 --- /dev/null +++ b/core/src/components/menu/test/a11y/index.html @@ -0,0 +1,41 @@ + + + + + + Segment - a11y + + + + + + + + + +
+

Menu

+ + + + Menu + + + + + + Button + + + Button 2 + + Menu Item + Menu Item + Menu Item + + + +
+ + + \ No newline at end of file diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts index 8f92edca6e..e2feb52617 100644 --- 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 */ diff --git a/core/src/components/menu/test/basic/index.html b/core/src/components/menu/test/basic/index.html index cbfc150651..021a2f8a53 100644 --- a/core/src/components/menu/test/basic/index.html +++ b/core/src/components/menu/test/basic/index.html @@ -24,7 +24,7 @@ - + Start Menu @@ -32,6 +32,9 @@ + + Button + Menu Item Menu Item Menu Item @@ -82,7 +85,7 @@ - Open Start Menu + Open Start Menu Open End Menu Open Custom Menu