fix(MobileMenuToggle): trap keyboard focus inside the mobile menu

[timothyjordan]

Description

Fixes #2697.

When the mobile menu is open on narrow viewports, Tab navigation currently escapes the menu after the last focusable element (the theme switcher) and lands on the underlying page content.

This PR extends the existing StarlightMenuButton custom element with a keydown handler that wraps Tab / Shift+Tab focus between the first and last visible focusable elements inside the surrounding <nav> while the menu is expanded (body[data-mobile-menu-expanded]).

The trap is scoped to the same <nav> already used for the Escape listener and is gated on the existing data attribute, so there is no behavioural change while the menu is closed.

Test plan

• Added Playwright e2e coverage in packages/starlight/__e2e__/mobile-menu.test.ts:
  • Tab from the last focusable wraps to the first.
  • Shift+Tab from the first wraps to the last.
  • Trap does not engage while the menu is closed.
  • Escape still closes the menu and restores focus to the toggle (regression check).
• All existing unit tests (519) and e2e tests pass locally.

Changeset

Patch bump for @astrojs/starlight included.

[changeset-bot]

🦋 Changeset detected

Latest commit: cb0c019

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@astrojs/starlight	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	cb0c019
🔍 Latest deploy log	https://app.netlify.com/projects/astro-starlight/deploys/6a0b9e35cd600b00087f3023
😎 Deploy Preview	https://deploy-preview-3911--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[astrobot-houston]

Hello! Thank you for opening your first PR to Starlight! ✨

Here’s what will happen next:

1. Our GitHub bots will run to check your changes.
   If they spot any issues you will see some error messages on this PR.
   Don’t hesitate to ask any questions if you’re not sure what these mean!

2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🤩

3. One or more of our maintainers will take a look and may ask you to make changes.
   We try to be responsive, but don’t worry if this takes a few days.

[HiDeoo]

Thanks for the contribution 🙌

At first glance, I notice a few potential issues with the current approach:

• The current code would exclude the close button from the focus loop, which feels a bit odd to me when quickly testing it out.
• The manually maintained list of interactive elements could be difficult to maintain. For example, right now it misses <summary> that we are using for sidebar groups. I think it would also need to be extended to include all possible interactive elements as users may add new ones in their own project.
• If we change the viewport width to a point where the mobile menu becomes hidden, e.g. changing the orientation on a tablet or resizing the window, the focus trap would still be active and prevent users from accessing the rest of the page.

Thinking outside of the box, I wonder if we could use the inert attribute to achieve a similar effect in this case. I think we would only need to set inert on the .main-frame and potentially the "Skip to content" link to achieve a similar effect without needing to maintain a list of interactive elements. This could end up being a tiny method called from setExpanded() and a media query listener to handle the viewport width changes.

What do you think? Would you be open to exploring this approach so we can discuss the pros and cons of each approach in more detail?

[HiDeoo]

We don't really have E2E tests right now for this part of the code or similar interactions. I'm not sure yet if we want to add some or not, but if we do, we would probably add them to the packages/starlight/__e2e__/basics.test.ts file to avoid rebuilding the basics fixture twice.

Maybe until we agree on the approach, we can omit them and add them later when we have a better idea of the final implementation and know for sure that we want to add E2E tests for this part of the code?