Skip to content

[menu] Implement WCAG-compliant focus management for disabled items - #2573

Closed
Ch-Valentine wants to merge 1 commit into
mui:masterfrom
Ch-Valentine:feat/menu-focus-management-wcag
Closed

[menu] Implement WCAG-compliant focus management for disabled items#2573
Ch-Valentine wants to merge 1 commit into
mui:masterfrom
Ch-Valentine:feat/menu-focus-management-wcag

Conversation

@Ch-Valentine

@Ch-Valentine Ch-Valentine commented Aug 24, 2025

Copy link
Copy Markdown

Summary

Implements proper focus management for disabled menu items according to WCAG accessibility guidelines.

This is related to #1733.

Changes

  • MenuRoot: Added disabledIndices state tracking and passed to useListNavigation
  • MenuItem: Added logic to register/unregister disabled state in the indices array
  • MenuRootContext: Updated interface to include disabled indices management
  • Tests: Added comprehensive focus management tests covering:
    • Keyboard opening with disabled first item (WCAG 2.1.1, 2.4.3)
    • Arrow key navigation skipping disabled items (WCAG 2.1.1, 2.4.3)

WCAG Compliance

  • WCAG 2.1.1 (Keyboard): All functionality available from keyboard, disabled items properly skipped
  • WCAG 2.4.3 (Focus Order): Logical focus order that skips non-focusable disabled items

Testing

  • All existing tests pass
  • New focus management tests added and passing
  • Manual testing confirms proper keyboard navigation behavior

…d items

- Add disabled indices tracking to MenuRoot
- Update MenuItem to register disabled state
- Add focus management tests for keyboard navigation
- Ensure disabled items are skipped during arrow key navigation
- Follow WCAG 2.1.1 (Keyboard) and WCAG 2.4.3 (Focus Order) guidelines
@pkg-pr-new

pkg-pr-new Bot commented Aug 24, 2025

Copy link
Copy Markdown

vite-css-base-ui-example

pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/react@2573
pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/utils@2573

commit: ff87fda

@mui-bot

mui-bot commented Aug 24, 2025

Copy link
Copy Markdown

Bundle size report

Bundle Parsed size Gzip size
@base-ui-components/react 🔺+311B(+0.10%) 🔺+113B(+0.11%)

Details of bundle changes

Generated by 🚫 dangerJS against ff87fda

@netlify

netlify Bot commented Aug 24, 2025

Copy link
Copy Markdown

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit ff87fda
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/68aabe7c1dd5990008eb6a2f
😎 Deploy Preview https://deploy-preview-2573--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@Ch-Valentine

Copy link
Copy Markdown
Author

Demo with a focus management problem BEFORE my changes: https://codesandbox.io/p/sandbox/intelligent-dew-gxwsrp

@atomiks

atomiks commented Aug 24, 2025

Copy link
Copy Markdown
Contributor

@Ch-Valentine the menubar pattern mentions disabled items should be focusable. Screen readers announce them as dimmed, which may be confusing if they're skipped.

In macOS, when using VoiceOver, they don't get skipped; while without VoiceOver, they do. This distinction not being as feasible to check on the web may be why the recommendation always recommends focusing disabled items.

Screenshot 2025-08-24 at 5 29 07 pm Screenshot 2025-08-24 at 5 32 53 pm

As for skipping the first disabled items when first opening, that may be correct regardless, though I am not positive

Screenshot 2025-08-24 at 5 32 04 pm

@Ch-Valentine

Copy link
Copy Markdown
Author

@atomiks thanks for the answer! Hm, that make sense as well) Should I close the PR ?

@atomiks

atomiks commented Aug 24, 2025

Copy link
Copy Markdown
Contributor

@Ch-Valentine it seems it should be skipping the first disabled items when first opening, but still allow them be navigated to with arrow keys afterwards based on the recommendation for menubar items

This issue highlights a somewhat related issue for tabs: #2098

@oliviertassinari oliviertassinari added the component: menu Changes related to the menu component. label Aug 24, 2025
@oliviertassinari

oliviertassinari commented Aug 24, 2025

Copy link
Copy Markdown
Member

This discussion reminds me of the disabledItemsFocusable prop in https://mui.com/material-ui/api/menu-list/#menu-list-prop-disabledItemsFocusable. The history: mui/material-ui#19967.

@oliviertassinari oliviertassinari changed the title [base-ui][Menu] Implement WCAG-compliant focus management for disabled items [menu] Implement WCAG-compliant focus management for disabled items Aug 24, 2025
@colmtuite

Copy link
Copy Markdown
Contributor

I addressed this in Discord recently.

Some stuff from ARIA docs from my Discord post.

"There are some contexts where it is common for disabled elements to be focusable, especially inside of composite widgets. For example, as demonstrated in the menu and menubar pattern, disabled items are focusable when navigating through a menu with the arrow keys."

https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#kbd_disabled_controls
https://www.w3.org/WAI/ARIA/apg/patterns/menubar/#:~:text=Disabled%20menu%20items%20are%20focusable%20but%20cannot%20be%20activated.
https://www.w3.org/WAI/ARIA/apg/patterns/toolbar/#:~:text=Typically%2C%20disabled%20elements%20are%20not%20focusable%20when%20navigating%20with%20a%20keyboard.

I'll close this PR since it would be a mistake. Imo, making it configurable would be a mistake too.

But yes @Ch-Valentine @atomiks it does seem we should be be skipping the first disabled items when first opening. I guess this should be implemented across many components.

@colmtuite colmtuite closed this Aug 24, 2025
@oliviertassinari

oliviertassinari commented Aug 24, 2025

Copy link
Copy Markdown
Member

What the other libraries seem to do:

So on those benchmarks, there is a clear skew in favor of not focusable. And on our side, we get user pressure in this same direction, by chronological order:

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

Labels

accessibility a11y component: menu Changes related to the menu component.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants