Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 14 commits into from
Apr 8, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f4471e1
fix(menu): preserve scroll position when focusing on open
Mar 31, 2022
99f58c1
test(menu): add more content to basic menu so it can scroll
Mar 31, 2022
75dffa0
test(menu): add scroll position test
Apr 1, 2022
47b4b49
chore(): lint
Apr 1, 2022
b955ce0
chore(): lint
Apr 1, 2022
2080f23
Merge branch 'main' into FW-634
Apr 1, 2022
20fd27f
fix(menu): revert changes and focus host el instead
Apr 6, 2022
239bdbf
test(menu): update focus trap tests to match new behavior
Apr 6, 2022
4769761
Merge branch 'main' into FW-634
Apr 6, 2022
f8857c7
Merge branch 'main' into FW-634
averyjohnston Apr 6, 2022
db81cd8
Merge branch 'main' into FW-634
liamdebeasi Apr 7, 2022
cac363d
fix(menu): only move focus if menu isn't already focused
Apr 8, 2022
833f2b9
Merge branch 'main' into FW-634
averyjohnston Apr 8, 2022
03c62a1
Update core/src/components/menu/menu.tsx
averyjohnston Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions core/src/components/menu/menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ export class Menu implements ComponentInterface, MenuI {
}

this.beforeAnimation(shouldOpen);

await this.loadAnimation();
await this.startAnimation(shouldOpen, animated);
this.afterAnimation(shouldOpen);
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions core/src/components/menu/test/basic/e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
liamdebeasi marked this conversation as resolved.
Show resolved Hide resolved
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
*/
Expand Down
15 changes: 15 additions & 0 deletions core/src/components/menu/test/basic/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions core/src/components/menu/test/focus-trap/e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/menu/test/focus-trap/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down