Skip to content

PLANET-8026 Trapped focus within mobile nav menu when open#2853

Open
Osong-Michael wants to merge 3 commits intomainfrom
main-nav-trap-focus-mobile
Open

PLANET-8026 Trapped focus within mobile nav menu when open#2853
Osong-Michael wants to merge 3 commits intomainfrom
main-nav-trap-focus-mobile

Conversation

@Osong-Michael
Copy link
Contributor

Summary

On mobile, the toggle correctly announces that the navigation is expanded, but focus does not move into the opened menu. The user remains on the page behind the overlay and cannot interact with the navigation. When the mobile menu opens, focus must move into it, and focus trap should be applied.


Ref: https://greenpeace-planet4.atlassian.net/browse/PLANET-8026

Testing

On mobile using a screen reader or tabbing with a keyboard, the focus is trapped within the open navigation menu until it is closed.

@Osong-Michael Osong-Michael self-assigned this Jan 16, 2026
@Osong-Michael Osong-Michael marked this pull request as ready for review January 16, 2026 10:12
@Osong-Michael Osong-Michael added Review UAT Needed This PR requires User Acceptance Tests before merge labels Jan 16, 2026
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Jan 16, 2026
/unhold deb7bb07-a8ce-4a26-93a6-ec777155ba74
@planet-4
Copy link
Contributor

planet-4 commented Jan 16, 2026

Test instance is ready 🚀

🌑 phoebe | admin | blocks report | CircleCI | composer-local.json

⌚ 2026.02.17 09:31:44

@Osong-Michael Osong-Michael force-pushed the main-nav-trap-focus-mobile branch from 0fbc4b2 to 2d2025d Compare January 21, 2026 11:42
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Jan 21, 2026
/unhold a0ebbbaf-fb37-4dd7-83d1-3e9cc963db56
@Osong-Michael Osong-Michael added UAT Passed User Acceptance Tests passed and removed UAT Needed This PR requires User Acceptance Tests before merge labels Jan 22, 2026
@Osong-Michael Osong-Michael requested a review from mleray January 22, 2026 11:21
@mleray
Copy link
Contributor

mleray commented Jan 27, 2026

On tablet when the tab focus goes to the logo in the burger menu, the page scrolls down which I don't think is expected?

screen.recording.mov

Edit: this also happens on mobile! You can also see the logo getting a bit bigger

Screenshot 2026-01-27 at 10 02 23

@mleray
Copy link
Contributor

mleray commented Jan 27, 2026

I have two different focus ring colors in the menu, do you see this as well? However it's on live sites too so it's not related to your changes, it can be handled in another PR.

Screenshot 2026-01-27 at 09 53 49 Screenshot 2026-01-27 at 09 54 19

@Osong-Michael
Copy link
Contributor Author

On tablet when the tab focus goes to the logo in the burger menu, the page scrolls down which I don't think is expected?

screen.recording.mov
Edit: this also happens on mobile! You can also see the logo getting a bit bigger

Screenshot 2026-01-27 at 10 02 23

There is already a ticket created to look into this @mleray

@mleray
Copy link
Contributor

mleray commented Jan 27, 2026

There is already a ticket created to look into this @mleray

@Osong-Michael I can't reproduce it on the GPI site though... and this isn't when closing the menu, it's when focusing on the logo within the menu.

@Osong-Michael
Copy link
Contributor Author

There is already a ticket created to look into this @mleray

@Osong-Michael I can't reproduce it on the GPI site though... and this isn't when closing the menu, it's when focusing on the logo within the menu.

So if you look at the ticket for this PR in Jira, I explained a bit more what was happening, don't know why but the ticket was created to look into it.

@mleray
Copy link
Contributor

mleray commented Jan 27, 2026

So if you look at the ticket for this PR in Jira, I explained a bit more what was happening, don't know why but the ticket was created to look into it.

@Osong-Michael but it seems it's related to your changes in this PR, because it's not happening in live sites or in main branch... so it would be nice to fix it here rather than merge it.

@Osong-Michael Osong-Michael requested a review from mleray January 27, 2026 14:15
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Jan 27, 2026
/unhold 85bc2875-46bb-4378-92d9-7384e56003d5
@Osong-Michael Osong-Michael force-pushed the main-nav-trap-focus-mobile branch from d72e661 to 283238b Compare February 9, 2026 08:47
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Feb 9, 2026
/unhold 3df92e05-da91-416a-84f2-07683cb2be41
@mleray
Copy link
Contributor

mleray commented Feb 9, 2026

When I exit the burger menu, the focus seems to move to the content but shouldn't it be in the navigation bar instead?

@mleray
Copy link
Contributor

mleray commented Feb 9, 2026

Maybe you should also have Houssam test it again since you fixed the issue of the page scrolling down.

@Osong-Michael
Copy link
Contributor Author

When I exit the burger menu, the focus seems to move to the content but shouldn't it be in the navigation bar instead?

I would assume not the case seeing the burger menu already contains the navigation elements, if the user is exiting out of that, most likely it is to continue to the content, that is just what I think.

@Osong-Michael Osong-Michael force-pushed the main-nav-trap-focus-mobile branch from 283238b to eb35a44 Compare February 10, 2026 09:38
@Osong-Michael Osong-Michael requested a review from mleray February 10, 2026 09:39
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Feb 10, 2026
/unhold e35762b0-314d-4449-8c3c-cd535c83686c
@mleray
Copy link
Contributor

mleray commented Feb 10, 2026

Let me know once Houssam has taken another look, then I can approve 👍

- Updated codebase to trap focus with mobile menu when open
- Refactor mobile nav focus trap logic
- Refactored selector to use spread operator instead of Array.from
@Osong-Michael Osong-Michael force-pushed the main-nav-trap-focus-mobile branch from eb35a44 to c892824 Compare February 17, 2026 09:23
planet-4 added a commit to greenpeace/planet4-test-phoebe that referenced this pull request Feb 17, 2026
/unhold 4e20bab4-b69b-463c-a12b-04a9e1a7bd96
const donateBtn = mobileNav.querySelector('.btn-donate');
const closeBtn = mobileNav.querySelector(NAV_MENU_CLOSE_CLASS);
const logo = mobileNav.querySelector(SITE_LOGO_CLASS);
const focusableSelectors =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a variable here? It's only used once

Copy link
Contributor

@mleray mleray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👏 I just left a small comment but it's not a blocker!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review [Test Env] phoebe UAT Passed User Acceptance Tests passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants