From da896848776105ba1f7035c4412495786199bade Mon Sep 17 00:00:00 2001
From: Amanda Smith <90629384+amandaesmith3@users.noreply.github.com>
Date: Fri, 8 Apr 2022 16:01:08 -0500
Subject: [PATCH] fix(menu): preserve scroll position when focusing on open
(#25044)
---
core/src/components/menu/menu.tsx | 14 ++++++---
core/src/components/menu/test/basic/e2e.ts | 31 ++++++++++++++++++-
.../src/components/menu/test/basic/index.html | 15 +++++++++
.../components/menu/test/focus-trap/e2e.ts | 4 +--
.../menu/test/focus-trap/index.html | 2 +-
5 files changed, 58 insertions(+), 8 deletions(-)
diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx
index e0d4091082..c30108dcf5 100644
--- 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
index 171d4cb590..dd83dfeacf 100644
--- a/core/src/components/menu/test/basic/e2e.ts
+++ b/core/src/components/menu/test/basic/e2e.ts
@@ -28,11 +28,40 @@ test('menu: focus trap', async () => {
await menu.waitForVisible();
let activeElID = await getActiveElementID(page);
- expect(activeElID).toEqual('start-menu-button');
+ 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);
});
/**
diff --git a/core/src/components/menu/test/basic/index.html b/core/src/components/menu/test/basic/index.html
index 6012e3a119..722be74614 100644
--- a/core/src/components/menu/test/basic/index.html
+++ b/core/src/components/menu/test/basic/index.html
@@ -49,6 +49,21 @@
Menu Item
Menu Item
Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
+ Menu Item
diff --git a/core/src/components/menu/test/focus-trap/e2e.ts b/core/src/components/menu/test/focus-trap/e2e.ts
index 0c06348292..af0a01211e 100644
--- 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
index e07ba0c0ff..0611466001 100644
--- a/core/src/components/menu/test/focus-trap/index.html
+++ b/core/src/components/menu/test/focus-trap/index.html
@@ -14,7 +14,7 @@
-
+