Skip to content

Commit 1686ecc

Browse files
timothyjordandelucisHiDeoo
authored
fix(MobileMenuToggle): trap keyboard focus inside the mobile menu (#3911)
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com> Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
1 parent cee1cc5 commit 1686ecc

4 files changed

Lines changed: 82 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@astrojs/starlight': patch
3+
---
4+
5+
Keeps keyboard focus inside the mobile menu while it is open, preventing focus moving to hidden interactive elements in page content.

packages/starlight/__e2e__/basics.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,75 @@ test.describe('ToC highlighting', () => {
772772
});
773773
});
774774

775+
test.describe('mobile menu focus trap', () => {
776+
test('traps focus within the mobile menu when open', async ({ page, getProdServer }) => {
777+
const starlight = await getProdServer();
778+
await page.setViewportSize({ width: 375, height: 667 });
779+
await starlight.goto('/headings');
780+
781+
const currentFocus = page.locator('*:focus');
782+
783+
// Open the mobile menu.
784+
const mobileMenuButton = page.getByRole('button', { name: 'Menu' });
785+
await mobileMenuButton.click();
786+
await expect(currentFocus).toHaveCount(1);
787+
788+
// Focus the theme selector, which is the last focusable element in the mobile menu.
789+
const themeSelector = page.getByRole('navigation', { name: 'Main' }).getByLabel('Select theme');
790+
await themeSelector.focus();
791+
await expect(currentFocus).toHaveCount(1);
792+
793+
// Tab out of the menu.
794+
await page.keyboard.press('Tab');
795+
796+
// Tabbing at the end of the mobile menu moves focus out of the viewport, so there should no
797+
// longer be any focused element.
798+
await expect(currentFocus).toHaveCount(0);
799+
800+
// Close the mobile menu.
801+
await mobileMenuButton.click();
802+
803+
// The focus trap should be released and tabbing will focus the mobile table of contents button.
804+
await page.keyboard.press('Tab');
805+
await expect(currentFocus).toHaveText(/On this page/);
806+
});
807+
808+
test('releases focus trap when the viewport resizes', async ({ page, getProdServer }) => {
809+
const starlight = await getProdServer();
810+
await page.setViewportSize({ width: 375, height: 667 });
811+
await starlight.goto('/anchor-heading');
812+
813+
const currentFocus = page.locator('*:focus');
814+
const anchorLinkAccessibleName = 'Section titled “An anchor heading”';
815+
const anchorHeadingLink = page.getByRole('link', { name: anchorLinkAccessibleName });
816+
817+
// Focus the anchor heading link to check it is focusable.
818+
await anchorHeadingLink.focus();
819+
await expect(currentFocus).toHaveText(anchorLinkAccessibleName);
820+
821+
// Open the mobile menu.
822+
const mobileMenuButton = page.getByRole('button', { name: 'Menu' });
823+
await mobileMenuButton.click();
824+
await expect(currentFocus).toHaveAccessibleName('Menu');
825+
826+
// Try to focus the anchor heading which should be prevented by the focus trap,
827+
// keeping focus where it is.
828+
await anchorHeadingLink.focus();
829+
await expect(currentFocus).toHaveAccessibleName('Menu');
830+
831+
// Resize the viewport to a wider size, which should release the focus trap.
832+
await page.setViewportSize({ width: 1280, height: 720 });
833+
834+
// The anchor heading link should be focusable again.
835+
await anchorHeadingLink.focus();
836+
await expect(currentFocus).toHaveText(anchorLinkAccessibleName);
837+
838+
// Resizing back to a smaller viewport should not re-enable the focus trap.
839+
await page.setViewportSize({ width: 375, height: 667 });
840+
await expect(currentFocus).toHaveText(anchorLinkAccessibleName);
841+
});
842+
});
843+
775844
/**
776845
* Loads the given `path` in a window of the specified `width` and `height` and checks that the
777846
* Starlight table of contents is highlighting an item with contents matching `pattern`.

packages/starlight/components/MobileMenuToggle.astro

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,18 @@ import Icon from '../user-components/Icon.astro';
2828
if (parentNav) {
2929
parentNav.addEventListener('keyup', (e) => this.closeOnEscape(e));
3030
}
31+
32+
// Reset menu state when the viewport toggles between mobile and desktop sizes.
33+
matchMedia('(min-width: 50em)').addEventListener('change', () => this.setExpanded(false));
3134
}
3235

3336
setExpanded(expanded: boolean) {
3437
this.setAttribute('aria-expanded', String(expanded));
3538
document.body.toggleAttribute('data-mobile-menu-expanded', expanded);
39+
// Trap keyboard focus inside the header and menu when the menu is expanded.
40+
document.querySelectorAll<HTMLElement>('.main-frame, .sl-skip-link').forEach((el) => {
41+
el.toggleAttribute('inert', expanded);
42+
});
3643
}
3744

3845
toggleExpanded() {

packages/starlight/components/SkipLink.astro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { PAGE_TITLE_ID } from '../constants';
33
---
44

5-
<a href={`#${PAGE_TITLE_ID}`}>{Astro.locals.t('skipLink.label')}</a>
5+
<a class="sl-skip-link" href={`#${PAGE_TITLE_ID}`}>{Astro.locals.t('skipLink.label')}</a>
66

77
<style>
88
@layer starlight.core {

0 commit comments

Comments
 (0)