From 632dafcd57de5086deebdc7d586b01710aa1a3ce Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 25 Jan 2022 10:09:37 -0500 Subject: [PATCH] fix(menu): focus trapping with menu and overlays no longer results in errors (#24611) resolves #24361, #24481 --- core/src/components/menu/menu.tsx | 17 +++++- .../components/menu/test/focus-trap/e2e.ts | 59 +++++++++++++++++++ .../menu/test/focus-trap/index.html | 56 ++++++++++++++++++ core/src/utils/overlays.ts | 14 ++++- 4 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 core/src/components/menu/test/focus-trap/e2e.ts create mode 100644 core/src/components/menu/test/focus-trap/index.html diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 3a1aded02a..56c642c235 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -7,6 +7,7 @@ import { getTimeGivenProgression } from '../../utils/animation/cubic-bezier'; import { GESTURE_CONTROLLER } from '../../utils/gesture'; import { assert, clamp, inheritAttributes, isEndSide as isEnd } from '../../utils/helpers'; import { menuController } from '../../utils/menu-controller'; +import { getOverlay } from '../../utils/overlays'; const iosEasing = 'cubic-bezier(0.32,0.72,0,1)'; const mdEasing = 'cubic-bezier(0.0,0.0,0.2,1)'; @@ -44,7 +45,21 @@ export class Menu implements ComponentInterface, MenuI { private inheritedAttributes: { [k: string]: any } = {}; - private handleFocus = (ev: Event) => this.trapKeyboardFocus(ev, document); + private handleFocus = (ev: FocusEvent) => { + /** + * Overlays have their own focus trapping listener + * so we do not want the two listeners to conflict + * with each other. If the top-most overlay that is + * open does not contain this ion-menu, then ion-menu's + * focus trapping should not run. + */ + const lastOverlay = getOverlay(document); + if (lastOverlay && !lastOverlay.contains(this.el)) { + return; + } + + this.trapKeyboardFocus(ev, document); + } @Element() el!: HTMLIonMenuElement; diff --git a/core/src/components/menu/test/focus-trap/e2e.ts b/core/src/components/menu/test/focus-trap/e2e.ts new file mode 100644 index 0000000000..82e6bb8478 --- /dev/null +++ b/core/src/components/menu/test/focus-trap/e2e.ts @@ -0,0 +1,59 @@ +import { newE2EPage } from '@stencil/core/testing'; + +const getActiveElementID = async (page) => { + const activeElement = await page.evaluateHandle(() => document.activeElement); + return await page.evaluate(el => el && el.id, activeElement); +} + +test('menu: focus trap with overlays', async () => { + const page = await newE2EPage({ + url: '/src/components/menu/test/focus-trap?ionic:_testing=true' + }); + + const ionDidOpen = await page.spyOnEvent('ionDidOpen'); + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss= await page.spyOnEvent('ionModalDidDismiss'); + + const menu = await page.find('ion-menu'); + await menu.callMethod('open'); + await ionDidOpen.next(); + + expect(await getActiveElementID(page)).toEqual('open-modal-button'); + + const openModal = await page.find('#open-modal-button'); + await openModal.click(); + await ionModalDidPresent.next(); + + expect(await getActiveElementID(page)).toEqual('modal-element'); + + const modal = await page.find('ion-modal'); + await modal.callMethod('dismiss'); + await ionModalDidDismiss.next(); + + expect(await getActiveElementID(page)).toEqual('open-modal-button'); +}); + +test('menu: focus trap with content inside overlays', async () => { + const page = await newE2EPage({ + url: '/src/components/menu/test/focus-trap?ionic:_testing=true' + }); + + const ionDidOpen = await page.spyOnEvent('ionDidOpen'); + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss= await page.spyOnEvent('ionModalDidDismiss'); + + const menu = await page.find('ion-menu'); + await menu.callMethod('open'); + await ionDidOpen.next(); + + expect(await getActiveElementID(page)).toEqual('open-modal-button'); + + const openModal = await page.find('#open-modal-button'); + await openModal.click(); + await ionModalDidPresent.next(); + + const button = await page.find('#other-button'); + await button.click(); + + expect(await getActiveElementID(page)).toEqual('other-button'); +}); diff --git a/core/src/components/menu/test/focus-trap/index.html b/core/src/components/menu/test/focus-trap/index.html new file mode 100644 index 0000000000..3f0aeb82d8 --- /dev/null +++ b/core/src/components/menu/test/focus-trap/index.html @@ -0,0 +1,56 @@ + + + + + Menu - Focus Trap + + + + + + + + + + + + Menu + + + + + Open Modal + + + + Modal content + Dismiss Modal + Other Button + + + + + +
+ + + Menu - Basic + + + + Open Menu + +
+ + + + diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts index 315d4c43ba..075b9e618a 100644 --- a/core/src/utils/overlays.ts +++ b/core/src/utils/overlays.ts @@ -295,7 +295,17 @@ const connectListeners = (doc: Document, options: OverlayListenerOptions) => { if (lastId === 0) { lastId = 1; if (options.trapKeyboardFocus) { - doc.addEventListener('focus', ev => trapKeyboardFocus(ev, doc), true); + doc.addEventListener('focus', (ev: FocusEvent) => { + /** + * ion-menu has its own focus trapping listener + * so we do not want the two listeners to conflict + * with each other. + */ + if (ev.target && (ev.target as HTMLElement).tagName === 'ION-MENU') { + return; + } + trapKeyboardFocus(ev, doc); + }, true); } // handle back-button click @@ -343,7 +353,7 @@ export const getOverlays = (doc: Document, selector?: string): HTMLIonOverlayEle * @param id The unique identifier for the overlay instance. * @returns The overlay element or `undefined` if no overlay element is found. */ -const getOverlay = (doc: Document, overlayTag?: string, id?: string): HTMLIonOverlayElement | undefined => { +export const getOverlay = (doc: Document, overlayTag?: string, id?: string): HTMLIonOverlayElement | undefined => { const overlays = getOverlays(doc, overlayTag).filter(o => !isOverlayHidden(o)); return (id === undefined) ? overlays[overlays.length - 1]