[menu] Remove invalid aria-owns owner span from menu portals#5002
[menu] Remove invalid aria-owns owner span from menu portals#5002lifeiscontent wants to merge 1 commit into
Conversation
lifeiscontent
requested review from
colmtuite,
flaviendelangle and
michaldudak
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Menu + FloatingPortal behavior to avoid rendering an aria-owns “owner” span that can violate ARIA child-role requirements, while keeping focus-guard behavior intact.
Changes:
- Disable the owner
span[aria-owns]forMenu.Portalby passingrenderOwner={false}toFloatingPortal. - Add a new
renderOwner?: booleanprop toFloatingPortal(defaulttrue) to control rendering of the owner node. - Add/extend tests for both
Menu.PortalandFloatingPortalto validate the new behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/react/src/menu/portal/MenuPortal.tsx | Stops rendering the owner node for menus by opting out via renderOwner={false}. |
| packages/react/src/menu/portal/MenuPortal.test.tsx | Adds a regression test asserting the owner aria-owns span is not present for menus. |
| packages/react/src/floating-ui-react/components/FloatingPortal.tsx | Introduces renderOwner prop and conditions owner node rendering on it. |
| packages/react/src/floating-ui-react/components/FloatingPortal.test.tsx | Adds tests to verify default owner rendering and opt-out behavior while keeping focus guards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
commit: |
Bundle size
PerformanceTotal duration: 1,270.90 ms -155.21 ms(-10.9%) | Renders: 50 (+0) | Paint: 1,921.26 ms -227.60 ms(-10.6%)
9 tests within noise — details Check out the code infra dashboard for more information about this PR. |
`Menu.Portal` rendered a visually hidden `<span aria-owns>` inside the menu/menubar, which axe flags under `aria-required-children`: a bare `span` is not an allowed child of the `menu`/`menubar` role. The span comes from `FloatingPortal`, where it keeps VoiceOver's virtual cursor in order for generic portaled panels. In a menu it is also redundant, since the trigger already exposes the popup through `aria-haspopup`, `aria-controls`, and `aria-expanded`. Add a `renderOwner` flag to `FloatingPortal` that drops only the owner span while leaving the focus guards and tab-order handling intact, and pass it from `Menu.Portal`. Other portals are unchanged.
lifeiscontent
force-pushed
the
fix/menu-aria-required-children
branch
from
bf0d6d4 to
a6e4e94
Compare
✅ Deploy Preview for base-ui ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for base-ui ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Closing this. Per the discussion in floating-ui/floating-ui#3424, the owner span isn't redundant after all: it places the portaled submenu correctly in the accessibility tree, so removing it for menus (what this PR does) would regress that placement rather than fix the underlying problem. The real gap is in axe-core's |
Fixes #4004
Menu.Portalrenders a visually hidden<span aria-owns>inside themenu/menubar. axe flags it under
aria-required-childrenbecause a barespanis not an allowed child of themenu/menubarrole(WAI-ARIA spec).
Root cause
The span comes from
FloatingPortal, which renders<span aria-owns={portalId}>to keep VoiceOver's virtual cursor in document order for generic portaled
panels. The span and the focus guards share one gate (
shouldRenderGuards),so the span can't be dropped without also losing tab-order management.
For a menu the span is redundant: the trigger already exposes the popup
through
aria-haspopup,aria-controls, andaria-expanded.Changes
renderOwnerflag toFloatingPortal(mirroring the existingrenderGuardsflag) that suppresses only the owner span, leaving thefocus guards and tab-order handling in place.
renderOwner={false}fromMenu.Portal. Submenus andMenubargo through the same portal, so they're covered too. Other portals are
unchanged and still render the span.
span[aria-owns], and thegeneric portal still does while keeping its focus guards.
Notes
This removes the node outright rather than hiding it with
aria-hidden(as in #4027). Adding
aria-hiddento the owner is contested upstream(floating-ui/floating-ui#3403) because it can defeat the virtual-cursor
ordering the span exists for. Since the span is redundant in menus,
dropping it avoids that trade-off.