fix(menu): focus trapping with menu and overlays no longer results in…

… errors (#24611) resolves #24361, #24481
ionic-team · Jan 25, 2022 · 632dafc · 632dafc
1 parent 88ce010
commit 632dafc
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 3 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8">
+    <title>Menu - Focus Trap</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no, viewport-fit=cover">
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet">
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet">
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+  </head>
+  <body>
+    <ion-app>
+      <ion-menu content-id="main">
+        <ion-header>
+          <ion-toolbar>
+            <ion-title>Menu</ion-title>
+          </ion-toolbar>
+        </ion-header>
+        <ion-content class="ion-padding">
+          <ion-item>
+            <ion-button id="open-modal-button">Open Modal</ion-button>
+          </ion-item>
+          <ion-modal id="modal-element" trigger="open-modal-button">
+            <ion-content class="ion-padding">
+              Modal content
+              <ion-button onclick="dismissModal()">Dismiss Modal</ion-button>
+              <ion-button id="other-button">Other Button</ion-button>
+            </ion-content>
+          </ion-modal>
+        </ion-content>
+      </ion-menu>
+
+      <div class="ion-page" id="main">
+        <ion-header>
+          <ion-toolbar>
+            <ion-title>Menu - Basic</ion-title>
+          </ion-toolbar>
+        </ion-header>
+        <ion-content class="ion-padding">
+          <ion-button onclick="openMenu()" id="open-menu-button">Open Menu</ion-button>
+        </ion-content>
+      </div>
+    </ion-app>
+    <script>
+      const menu = document.querySelector('ion-menu');
+      const modal = document.querySelector('ion-modal');
+      const openMenu = () => {
+        menu.open();
+      }
+      const dismissModal = () => {
+        modal.dismiss();
+      }
+    </script>
+  </body>
+</html>
diff --git 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]