fix(menu): preserve scroll position when focusing on open (#25044)

ionic-team · Apr 8, 2022 · da89684 · da89684
1 parent 3f3a2bc
commit da89684
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 7 deletions.
diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx
@@ -398,6 +398,7 @@ export class Menu implements ComponentInterface, MenuI {
     }
 
     this.beforeAnimation(shouldOpen);
+
     await this.loadAnimation();
     await this.startAnimation(shouldOpen, animated);
     this.afterAnimation(shouldOpen);
@@ -619,12 +620,17 @@ export class Menu implements ComponentInterface, MenuI {
       // emit open event
       this.ionDidOpen.emit();
 
-      // focus menu content for screen readers
-      if (this.menuInnerEl) {
-        this.focusFirstDescendant();
+      /**
+       * Move focus to the menu to prepare focus trapping, as long as
+       * it isn't already focused. Use the host element instead of the
+       * first descendant to avoid the scroll position jumping around.
+       */
+      const focusedMenu = document.activeElement?.closest('ion-menu');
+      if (focusedMenu !== this.el) {
+        this.el.focus();
       }
 
-      // setup focus trapping
+      // start focus trapping
       document.addEventListener('focus', this.handleFocus, true);
     } else {
       // remove css classes and unhide content from screen readers

diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts
@@ -28,13 +28,42 @@ 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');
 
+  // do it again to make sure focus stays inside menu
   await page.keyboard.press('Tab');
   activeElID = await getActiveElementID(page);
   expect(activeElID).toEqual('start-menu-button');
 });
 
+test('menu: preserve scroll position', 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();
+
+  await page.$eval('#start-menu ion-content', (menuContentEl: any) => {
+    return menuContentEl.scrollToPoint(0, 200);
+  });
+
+  await menu.callMethod('close');
+
+  await page.click('#open-first');
+  await menu.waitForVisible();
+
+  const scrollTop = await page.$eval('#start-menu ion-content', async (menuContentEl: any) => {
+    const contentScrollEl = await menuContentEl.getScrollElement();
+    return contentScrollEl.scrollTop;
+  });
+
+  expect(scrollTop).toEqual(200);
+});
+
 /**
  * RTL Tests
  */

diff --git a/core/src/components/menu/test/basic/index.html b/core/src/components/menu/test/basic/index.html
@@ -49,6 +49,21 @@
             <ion-item>Menu Item</ion-item>
             <ion-item>Menu Item</ion-item>
             <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</ion-item>
+            <ion-item>Menu Item</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>

diff --git a/core/src/components/menu/test/focus-trap/e2e.ts b/core/src/components/menu/test/focus-trap/e2e.ts
@@ -18,7 +18,7 @@ test('menu: focus trap with overlays', async () => {
   await menu.callMethod('open');
   await ionDidOpen.next();
 
-  expect(await getActiveElementID(page)).toEqual('open-modal-button');
+  expect(await getActiveElementID(page)).toEqual('menu');
 
   const openModal = await page.find('#open-modal-button');
   await openModal.click();
@@ -45,7 +45,7 @@ test('menu: focus trap with content inside overlays', async () => {
   await menu.callMethod('open');
   await ionDidOpen.next();
 
-  expect(await getActiveElementID(page)).toEqual('open-modal-button');
+  expect(await getActiveElementID(page)).toEqual('menu');
 
   const openModal = await page.find('#open-modal-button');
   await openModal.click();

diff --git a/core/src/components/menu/test/focus-trap/index.html b/core/src/components/menu/test/focus-trap/index.html
@@ -14,7 +14,7 @@
   </head>
   <body>
     <ion-app>
-      <ion-menu content-id="main">
+      <ion-menu content-id="main" id="menu">
         <ion-header>
           <ion-toolbar>
             <ion-title>Menu</ion-title>