Problem/Motivation

This is a followup to #3443571-36: Mobile version of Navigation should have focus trap, but it should be released along with that issue.

From @katannshaw:

keyboard focus must be trapped within it until the user:

* Selects the Close/OK/X button
* Selects the ESC key
* Clicks outside of the menu with their mouse

From @rkoller:

the close button should be reachable by keyboard same as there should be an option to close the mobile navigation by pressing the ESC key. but from my perspective if moved to a follow up those followup(s) and this issue should go into the same new version of drupal.

Setting to Major priority because #3443571: Mobile version of Navigation should have focus trap is Major.

WCAG success criteria

Steps to reproduce

  1. Enable the Navigation module.
  2. Log in as admin.
  3. Reduce the viewport width until the Navigation "Expand sidebar" button appears at top-left.
  4. Activate the "Expand sidebar" button to open the sidebar.
  5. If focus is not on an item within the sidebar, press the TAB key until focus is within the sidebar.
  6. Press the ESC key.
Expected behavior
  • The sidebar closes.
  • Focus returns to the "Expand sidebar" button.
Actual behavior
  • The sidebar does not close.
  • Focus remains within the sidebar.

Proposed resolution

If on the first level when pressing ESC it should close the toolbar and set focus on the "Expand sidebar" button. When hitting ESC on a popover it should keep the toolbar open but go back one level.

Remaining tasks

User interface changes

The "mobile" sidebar implements the expected behavior.

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3572628

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kentr created an issue. See original summary.

smustgrave’s picture

smustgrave commented
Status: Needs review » Active

Wrong status?

kentr’s picture

kentr commented

Wrong status?

Yes. This started as a clone of another issue, and I neglected to change the status.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

In discussion with @katannshaw, @mgifford, @mherchel, and myself, we considered this follow up to the focus trap issue as a blocker in #3391723: [PLAN] Accessibility review for new Navigation bar

p.s. not sure if we should keep the focus trap issue as the parent or better move it to related issues and make the a11y meta the parent instead?

kentr’s picture

kentr commented
Parent issue: #3443571: Mobile version of Navigation should have focus trap » #3391723: [PLAN] Accessibility review for new Navigation bar
Related issues: +#3443571: Mobile version of Navigation should have focus trap

p.s. not sure if we should keep the focus trap issue as the parent or better move it to related issues and make the a11y meta the parent instead?

Yes. I did that to tie this to the focus trap issue, but y'all made it an official higher-level blocker.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Postponed

Postponing this on #3443571: Mobile version of Navigation should have focus trap. It'll touch the same bit of JS, and will have conflicts.

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Active

#3443571: Mobile version of Navigation should have focus trap is in.

smustgrave’s picture

smustgrave commented

So the escape button does do something but only if you're in a popover. So should the escape key no longer close any popovers and instead close the entire toolbar?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Issue tags: +fldc26

We'll work on this in the contribution room at Florida DrupalCamp 2026 in a couple days, if it still needs it.

smustgrave’s picture

smustgrave commented
Issue tags: +a11y-bugsmash

Tagged as the latest a11y-bugsmash "one"

kentr’s picture

kentr commented

So the escape button does do something but only if you're in a popover. So should the escape key no longer close any popovers and instead close the entire toolbar?

How does one see the popover?

I'm not sure that I'm interpreting the question correctly, so this may be off-base. To me, ESC should close the current user-opened content. So if ESC closes the popover and returns the user to the sidebar, then another ESC should close the sidebar.

smustgrave’s picture

smustgrave commented

So on mobile if you open up a sub menu (in the code it calls them popovers) then esc takes you back one.

kentr’s picture

kentr commented

@smustgrave

Ok. Thanks.

To me, ESC should still close the sub-menu. But then if a submenu isn't open, ESC should close the sidebar.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

To me, ESC should still close the sub-menu. But then if a submenu isn't open, ESC should close the sidebar.

I agree with this! Look for a MR this Sunday (at the FLDC contribution sprint).

smustgrave’s picture

smustgrave commented
Issue summary: View changes

Updated the summary! Thanks everyone

kentr’s picture

kentr commented

Just a note that this is related to closing the sub-menu / popover with ESC: #3541741: [PP-1] Close only lowest-level submenu with Escape key

Not suggesting that it be fixed with this issue, just giving a heads up that there may be crossover in the code and some work was done on that issue in case it's useful / important here.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Issue tags: -fldc26

Note I was too busy mentoring at FLDC to get to this. I'm still planning on working on it this week or early next. But if someone else wants to get into it, please do so, and then I can just review :)

kentr’s picture

kentr commented

I'll take a stab at it.

kentr’s picture

kentr commented
Issue summary: View changes

Have made some progress, hopefully will create an MR soon.

I'm including some code to move focus to the "Expand sidebar" button when closed by ESC due to the following snippet from SC 2.4.11 Focus Not Obscured (Minimum) (Level AA), as well as #19 by @cwilcox808 in #3443571-19: Mobile version of Navigation should have focus trap.

When the additional content is opened, it takes focus and the tab ring is constrained to the new content until it is dismissed. This modality is somewhat like a dialog, in that a user cannot navigate beyond the opened content by keyboard without dismissing it first (typically by pressing Esc). However, unlike in a modal dialog, in some implementations a pointer user may be able to interact with content outside the opened section without dismissing it. Since this pattern potentially creates an inequitable experience between keyboard and pointer users, it should be used cautiously. That said, it does prevent the opened content from obscuring the keyboard focus in the main content, and thus should pass this SC. This is described and demonstrated in a short video in the Knowbility article in the reference section, under the section heading Keep keyboard focus in the slide-out navigation until it's closed.

This also will be consistent with the behavior for the lower-level menus / sub-menus. It was in the Steps to reproduce > Expected behavior, but not in the proposed resolution.

kentr opened merge request !14935

kentr’s picture

kentr commented
Status: Active » Needs review
Issue tags: -Needs tests

This is ready for review.

In my last comment, I think I conflated the snippet with the Knowbilty article that it references. The article discusses moving focus back to the navigation trigger.

For a problem that I encountered as part of the fix, I also used changes from #3541741: [PP-1] Close only lowest-level submenu with Escape key.

If those changes are accepted as part of this issue, @hinal05 should be credited here. Also, it should fix #3541741: [PP-1] Close only lowest-level submenu with Escape key.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Needs work

Commented on the MR

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Reviewed & tested by the community

Thank you for addressing the feedback! Tested this again manually and this works great!

  • catch committed cffd75be on 11.x 
    fix: #3572628 Mobile sidebar should close by ESCAPE key

By: kentr
By:...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

I'm not really qualified to review the js here, especially the nightwatch tests, but since @lauriii RTBCed and the approach has good consensus (and this is the last high priority navigation accessibility issue), going to go ahead here.

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 302f9077 on main 
    fix: #3572628 Mobile sidebar should close by ESCAPE key

By: kentr
By:...

catch closed merge request !14935

kentr’s picture

kentr commented

I think we also need to credit @hinal05. I used her code from #3541741: [PP-1] Close only lowest-level submenu with Escape key.

Unless she'll get credit on that issue? I'll check to see if it's fixed now.

catch’s picture

catch
he/him
Primary language English
commented

I've marked that issue as outdated and applied credit over there.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Should this be marked as Fixed?

kentr’s picture

kentr commented
Status: Reviewed & tested by the community » Fixed

@catch marked it as fixed in #25. Somehow it appears to have reverted.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

kentr’s picture

kentr commented
Version: main » 11.1.x-dev

Apparently, "somehow" was me, with #29.

Also restoring the version that @catch set in #25.

kentr’s picture

kentr commented
Version: 11.1.x-dev » 11.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.