From 579e710c4794428d1882b705a62cebc3b10b1dc4 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 13 Oct 2021 12:53:27 -0500 Subject: [PATCH 01/20] fix(menu): add basic accessibility features --- core/src/components/menu/menu.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 452125a29a5..96d4270ee72 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -246,6 +246,13 @@ AFTER: } } + @Listen('keydown') + onKeydown(ev: KeyboardEvent) { + if(ev.key === 'Escape') { + this.close(); + } + } + /** * Returns `true` is the menu is open. */ @@ -512,6 +519,11 @@ AFTER: // emit open event this.ionDidOpen.emit(); + + // focus menu content for screen readers + if (this.menuInnerEl) { + this.menuInnerEl.focus(); + } } else { // remove css classes this.el.classList.remove(SHOW_MENU); @@ -576,13 +588,14 @@ AFTER: 'menu-pane-visible': isPaneVisible }} > -
this.menuInnerEl = el} > -
+ this.backdropEl = el} From e51440b0318d86405d9d00af247be306da4150e0 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Thu, 14 Oct 2021 12:45:03 -0500 Subject: [PATCH 02/20] fix(menu): add focus trapping --- core/src/components/menu/menu.tsx | 74 ++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 96d4270ee72..c56842d94f8 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -12,6 +12,7 @@ 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 = 'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input[type="text"]:not([disabled]), input[type="radio"]:not([disabled]), input[type="checkbox"]:not([disabled]), select:not([disabled])'; /** * @part container - The container for the menu content. @@ -39,6 +40,9 @@ export class Menu implements ComponentInterface, MenuI { backdropEl?: HTMLElement; menuInnerEl?: HTMLElement; contentEl?: HTMLElement; + lastFocus?: HTMLElement; + + private handleFocus?: any; @Element() el!: HTMLIonMenuElement; @@ -308,6 +312,65 @@ AFTER: return menuController._setOpen(this, shouldOpen, animated); } + private focusFirstDescendant() { + const { el, menuInnerEl } = this; + const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null; + + if(firstInput) { + firstInput.focus(); + } else if(menuInnerEl) { + menuInnerEl.focus(); + } + } + + private focusLastDescendant() { + const { el, menuInnerEl } = this; + const inputs = Array.from(el.querySelectorAll(focusableQueryString)); + const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null; + + if(lastInput) { + lastInput.focus(); + } else if(menuInnerEl) { + menuInnerEl.focus(); + } + } + + private trapKeyboardFocus(ev: Event, doc: Document) { + const target = ev.target as HTMLElement | null; + if(!this.el || !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) { @@ -522,8 +585,12 @@ AFTER: // focus menu content for screen readers if (this.menuInnerEl) { - this.menuInnerEl.focus(); + this.focusFirstDescendant(); } + + // setup focus trapping + this.handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document); + document.addEventListener('focus', this.handleFocus, true); } else { // remove css classes this.el.classList.remove(SHOW_MENU); @@ -540,6 +607,10 @@ AFTER: // emit close event this.ionDidClose.emit(); + + // undo focus trapping so multiple menus don't collide + document.removeEventListener('focus', this.handleFocus, true); + this.handleFocus = undefined; } } @@ -579,6 +650,7 @@ AFTER: return ( Date: Thu, 14 Oct 2021 12:45:46 -0500 Subject: [PATCH 03/20] test(menu): add test for focus trapping --- core/src/components/menu/test/basic/e2e.ts | 25 +++++++++++++++++++ .../src/components/menu/test/basic/index.html | 5 +++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts index 8f92edca6e7..1a9cd43cd61 100644 --- a/core/src/components/menu/test/basic/e2e.ts +++ b/core/src/components/menu/test/basic/e2e.ts @@ -1,6 +1,12 @@ import { testMenu } from '../test.utils'; +import { newE2EPage } from '@stencil/core/testing'; +import { expectFiles } from '@stencil/core/testing/testing-utils'; 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 +20,25 @@ 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'); + + await page.keyboard.press('Tab'); + activeElID = await getActiveElementID(page); + expect(activeElID).toEqual('start-menu-button'); + + await page.keyboard.press('Tab'); + activeElID = await getActiveElementID(page); + expect(activeElID).toEqual('start-menu'); +}); + /** * RTL Tests */ diff --git a/core/src/components/menu/test/basic/index.html b/core/src/components/menu/test/basic/index.html index cbfc1506511..c332ada6e7d 100644 --- a/core/src/components/menu/test/basic/index.html +++ b/core/src/components/menu/test/basic/index.html @@ -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 From 2fef4537196fc710d24623a70eada0339485578f Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Thu, 14 Oct 2021 12:49:58 -0500 Subject: [PATCH 04/20] style(menu): lint fixes --- core/src/components/menu/menu.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index c56842d94f8..568cf266e15 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -252,7 +252,7 @@ AFTER: @Listen('keydown') onKeydown(ev: KeyboardEvent) { - if(ev.key === 'Escape') { + if (ev.key === 'Escape') { this.close(); } } @@ -316,9 +316,9 @@ AFTER: const { el, menuInnerEl } = this; const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null; - if(firstInput) { + if (firstInput) { firstInput.focus(); - } else if(menuInnerEl) { + } else if (menuInnerEl) { menuInnerEl.focus(); } } @@ -328,16 +328,16 @@ AFTER: const inputs = Array.from(el.querySelectorAll(focusableQueryString)); const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null; - if(lastInput) { + if (lastInput) { lastInput.focus(); - } else if(menuInnerEl) { + } else if (menuInnerEl) { menuInnerEl.focus(); } } private trapKeyboardFocus(ev: Event, doc: Document) { const target = ev.target as HTMLElement | null; - if(!this.el || !target) return; + if (!target) { return; } /** * If the target is inside the menu contents, let the browser @@ -365,7 +365,7 @@ AFTER: * already and pressed Shift + Tab, so we need to wrap to the * last descendant. */ - if(this.lastFocus === doc.activeElement) { + if (this.lastFocus === doc.activeElement) { this.focusLastDescendant(); } } From 56e1e44bf385808b697d38be65f7a71e12503152 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 15 Oct 2021 15:06:55 -0500 Subject: [PATCH 05/20] fix(menu): focus first element inside instead of whole menu --- core/src/components/menu/menu.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 568cf266e15..f9a3a1dd96a 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -12,7 +12,7 @@ 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 = 'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input[type="text"]:not([disabled]), input[type="radio"]:not([disabled]), input[type="checkbox"]:not([disabled]), select:not([disabled])'; +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. @@ -650,7 +650,6 @@ AFTER: return ( this.menuInnerEl = el} > From a4cda658f44b6fef265e68e260cc1f2ae62acbc9 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 15 Oct 2021 15:37:02 -0500 Subject: [PATCH 06/20] test(menu): fix focus trap test to account for new behavior --- core/src/components/menu/test/basic/e2e.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts index 1a9cd43cd61..81e228579dd 100644 --- a/core/src/components/menu/test/basic/e2e.ts +++ b/core/src/components/menu/test/basic/e2e.ts @@ -28,15 +28,11 @@ test('menu: focus trap', async () => { await menu.waitForVisible(); let activeElID = await getActiveElementID(page); - expect(activeElID).toEqual('start-menu'); - - await page.keyboard.press('Tab'); - activeElID = await getActiveElementID(page); expect(activeElID).toEqual('start-menu-button'); await page.keyboard.press('Tab'); activeElID = await getActiveElementID(page); - expect(activeElID).toEqual('start-menu'); + expect(activeElID).toEqual('start-menu-button'); }); /** From bdc32011dcd63d77a5c387019c37c7bbcd21d40e Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 15 Oct 2021 15:41:02 -0500 Subject: [PATCH 07/20] refactor(menu): pull focus handler into its own prop --- core/src/components/menu/menu.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index f9a3a1dd96a..389ba529031 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -42,7 +42,7 @@ export class Menu implements ComponentInterface, MenuI { contentEl?: HTMLElement; lastFocus?: HTMLElement; - private handleFocus?: any; + private handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document); @Element() el!: HTMLIonMenuElement; @@ -589,7 +589,6 @@ AFTER: } // setup focus trapping - this.handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document); document.addEventListener('focus', this.handleFocus, true); } else { // remove css classes @@ -610,7 +609,6 @@ AFTER: // undo focus trapping so multiple menus don't collide document.removeEventListener('focus', this.handleFocus, true); - this.handleFocus = undefined; } } From e23f5f3d186de8b29069af348270501b528ce9ca Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 18 Oct 2021 10:02:41 -0500 Subject: [PATCH 08/20] test(menu): add a11y testing --- core/src/components/menu/test/a11y/e2e.ts | 15 +++++++ core/src/components/menu/test/a11y/index.html | 41 +++++++++++++++++++ 2 files changed, 56 insertions(+) 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/menu/test/a11y/e2e.ts b/core/src/components/menu/test/a11y/e2e.ts new file mode 100644 index 00000000000..364409a477e --- /dev/null +++ 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); +}); \ 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 00000000000..036277bc139 --- /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 From 51d32acc659fceaa7569c535f5fabfa3dd09b290 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 18 Oct 2021 13:02:13 -0500 Subject: [PATCH 09/20] fix(menu): prevent nested aria landmark from header inside menu --- core/src/components/menu/menu.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 389ba529031..29da6c23b3c 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -163,6 +163,13 @@ export class Menu implements ComponentInterface, MenuI { const el = this.el; const parent = el.parentNode as any; + + // prevent nested aria landmarks + const headers = el.querySelectorAll('ion-header'); + if (headers.length > 0) { + headers.forEach(head => head.setAttribute('role', 'none')); + } + if (this.contentId === undefined) { console.warn(`[DEPRECATED][ion-menu] Using the [main] attribute is deprecated, please use the "contentId" property instead: BEFORE: @@ -648,6 +655,7 @@ AFTER: return ( Date: Tue, 19 Oct 2021 14:01:08 -0500 Subject: [PATCH 10/20] fix(menu): revert switch to nav element --- core/src/components/menu/menu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 29da6c23b3c..3cce8d33ea0 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -665,13 +665,13 @@ AFTER: 'menu-pane-visible': isPaneVisible }} > - + this.backdropEl = el} From 150a7ace42f4d2a4635ad33277071f425bc16711 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Tue, 19 Oct 2021 14:17:42 -0500 Subject: [PATCH 11/20] fix(menu): remove unnecessary import from test --- core/src/components/menu/test/basic/e2e.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts index 81e228579dd..e2feb526174 100644 --- a/core/src/components/menu/test/basic/e2e.ts +++ b/core/src/components/menu/test/basic/e2e.ts @@ -1,6 +1,5 @@ import { testMenu } from '../test.utils'; import { newE2EPage } from '@stencil/core/testing'; -import { expectFiles } from '@stencil/core/testing/testing-utils'; const DIRECTORY = 'basic'; const getActiveElementID = async (page) => { From 079b62f267583388e0f025de624048715123d352 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Oct 2021 08:15:45 -0500 Subject: [PATCH 12/20] fix(menu): allow for custom aria-label --- core/src/components/menu/menu.tsx | 12 +++++++++--- core/src/components/menu/test/basic/index.html | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 3cce8d33ea0..871909059d9 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -5,7 +5,7 @@ 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)'; @@ -42,6 +42,8 @@ export class Menu implements ComponentInterface, MenuI { contentEl?: HTMLElement; lastFocus?: HTMLElement; + private inheritedAttributes: { [k: string]: any } = {}; + private handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document); @Element() el!: HTMLIonMenuElement; @@ -216,6 +218,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(); @@ -649,13 +655,13 @@ AFTER: } render() { - const { isEndSide, type, disabled, isPaneVisible } = this; + const { isEndSide, type, disabled, isPaneVisible, inheritedAttributes } = this; const mode = getIonMode(this); return ( - + Start Menu From f071d7dc764216b86933415f1536c9b91a3eefb2 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Oct 2021 08:26:23 -0500 Subject: [PATCH 13/20] fix(menu): move nested ARIA role logic to header for flexibility --- core/src/components/header/header.tsx | 6 +++++- core/src/components/menu/menu.tsx | 6 ------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/src/components/header/header.tsx b/core/src/components/header/header.tsx index 825c690ad37..c538a7b5b53 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 ( 0) { - headers.forEach(head => head.setAttribute('role', 'none')); - } - if (this.contentId === undefined) { console.warn(`[DEPRECATED][ion-menu] Using the [main] attribute is deprecated, please use the "contentId" property instead: BEFORE: From 045afe5e6ca89261cc47721e8815d461d78c72a1 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Oct 2021 09:06:59 -0500 Subject: [PATCH 14/20] fix(item): only add focusable class if it actually is focusable --- core/src/components/item/item.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 1ed8aa6bd3e..76acddc4b0b 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -153,6 +153,14 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac } } + componentDidRender() { + // up here instead of in render() because we need to wait for + // children to fully render too, so their classes are available + if (this.isFocusable()) { + this.el.classList.add('ion-focusable'); + } + } + componentDidUpdate() { // Do not use @Listen here to avoid making all items // appear as clickable to screen readers @@ -217,6 +225,11 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac return (this.isClickable() || this.hasCover()); } + private isFocusable(): boolean { + const focusableChildren = this.el.querySelectorAll('.ion-focusable'); + return (this.canActivate() || focusableChildren.length > 0); + } + private getFirstInput(): HTMLIonInputElement | HTMLIonTextareaElement { const inputs = this.el.querySelectorAll('ion-input, ion-textarea') as NodeListOf; return inputs[0]; @@ -289,7 +302,6 @@ 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, }) }} > From c30239a1f934ee5f759d09f5c458a9a19e3e8091 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Wed, 20 Oct 2021 09:07:48 -0500 Subject: [PATCH 15/20] fix(menu): allow focusing of menu itself, for a11y on menus with no focusable children --- core/src/components/menu/menu.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index be11f55405e..d2d190a69d0 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -668,6 +668,7 @@ AFTER:
this.menuInnerEl = el} > From ede41b0d2e200b076322a34020e77c3f9cb811f7 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 22 Oct 2021 14:33:00 -0500 Subject: [PATCH 16/20] fix(item): move isFocusable logic to state for better reactivity --- core/src/components/item/item.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 76acddc4b0b..5a627a5f331 100644 --- 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. @@ -153,14 +154,6 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac } } - componentDidRender() { - // up here instead of in render() because we need to wait for - // children to fully render too, so their classes are available - if (this.isFocusable()) { - this.el.classList.add('ion-focusable'); - } - } - componentDidUpdate() { // Do not use @Listen here to avoid making all items // appear as clickable to screen readers @@ -181,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 @@ -302,6 +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': this.focusable }) }} > From c334776ddd770c7e2c1637326747901b326213f9 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Fri, 22 Oct 2021 14:35:25 -0500 Subject: [PATCH 17/20] perf(item): only grab one focusable child --- core/src/components/item/item.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 5a627a5f331..cdff5749df6 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -222,8 +222,8 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac } private isFocusable(): boolean { - const focusableChildren = this.el.querySelectorAll('.ion-focusable'); - return (this.canActivate() || focusableChildren.length > 0); + const focusableChild = this.el.querySelector('.ion-focusable'); + return (this.canActivate() || focusableChild !== null); } private getFirstInput(): HTMLIonInputElement | HTMLIonTextareaElement { From 9066592fcd34e1452419f05b8dbb08fe0cbfbcd8 Mon Sep 17 00:00:00 2001 From: amandaesmith3 Date: Mon, 25 Oct 2021 15:49:15 -0500 Subject: [PATCH 18/20] fix(menu): hide page content from screen readers when menu is open --- core/src/components/menu/menu.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index d2d190a69d0..6a34ec9fa08 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -582,9 +582,10 @@ 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); + this.contentEl.setAttribute('aria-hidden', 'true'); } // emit open event @@ -598,11 +599,13 @@ AFTER: // 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); if (this.contentEl) { this.contentEl.classList.remove(MENU_CONTENT_OPEN); + this.contentEl.removeAttribute('aria-hidden'); } + if (this.backdropEl) { this.backdropEl.classList.remove(SHOW_BACKDROP); } From 3ba672a1137bfe17a63b8037a402de492a0d97a2 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 26 Oct 2021 20:39:39 +0000 Subject: [PATCH 19/20] fix(menu): fallback to focusing host element --- core/src/components/menu/menu.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 6a34ec9fa08..55c05879aff 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -320,25 +320,25 @@ AFTER: } private focusFirstDescendant() { - const { el, menuInnerEl } = this; + const { el } = this; const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null; if (firstInput) { firstInput.focus(); - } else if (menuInnerEl) { - menuInnerEl.focus(); + } else { + el.focus(); } } private focusLastDescendant() { - const { el, menuInnerEl } = this; + 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 if (menuInnerEl) { - menuInnerEl.focus(); + } else { + el.focus(); } } @@ -556,6 +556,7 @@ 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); + this.el.setAttribute('tabindex', '0'); if (this.backdropEl) { this.backdropEl.classList.add(SHOW_BACKDROP); } @@ -601,6 +602,7 @@ AFTER: } else { // remove css classes and unhide content from screen readers this.el.classList.remove(SHOW_MENU); + this.el.removeAttribute('tabindex'); if (this.contentEl) { this.contentEl.classList.remove(MENU_CONTENT_OPEN); this.contentEl.removeAttribute('aria-hidden'); @@ -671,7 +673,6 @@ AFTER:
this.menuInnerEl = el} > From 5782e2665f71d97d95df2e9c055e8ca075202a8f Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 26 Oct 2021 16:58:10 -0400 Subject: [PATCH 20/20] docs(menu): add comments --- core/src/components/menu/menu.tsx | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 55c05879aff..fe689a66c76 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -556,6 +556,15 @@ 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); @@ -586,6 +595,15 @@ AFTER: // 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'); } @@ -602,9 +620,20 @@ AFTER: } else { // 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'); }