Skip to content

fix(Menu): correct roles on GroupHeading and Separator - #2091

Open
LukianovII wants to merge 1 commit into
huntabyte:mainfrom
LukianovII:fix/menu-heading-separator-roles
Open

fix(Menu): correct roles on GroupHeading and Separator#2091
LukianovII wants to merge 1 commit into
huntabyte:mainfrom
LukianovII:fix/menu-heading-separator-roles

Conversation

@LukianovII

Copy link
Copy Markdown

Fixes #2090.

Menu.GroupHeading and Menu.Separator both render role="group", so a menu that uses them ends up with empty groups in the accessibility tree — a group whose only content is a heading, and another whose only content is a divider.

https://github.com/huntabyte/bits-ui/blob/main/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts#L1630
https://github.com/huntabyte/bits-ui/blob/main/packages/bits-ui/src/lib/bits/menu/menu.svelte.ts#L1658

Both look like copy-paste from MenuGroupState right above them.

The fix follows conventions already in this repo

  • GroupHeading → no role. SelectGroupHeadingState and CommandGroupHeadingState set only an id, and that is enough: the heading names its group through the group's aria-labelledby, which references it by id and does not depend on a role.
  • Separatorrole="separator". The top-level SeparatorRootState already resolves decorative ? "none" : "separator", so the menu one was the odd one out.

MenuGroupState, MenuRadioGroupState and MenuCheckboxGroupState keep role="group" — correct for those.

Testing

Added a case to dropdown-menu.browser.test.ts asserting the group is the only group, the separator is a separator, the heading carries no role, and the heading still names its group via aria-labelledby. Reverting either line of the fix turns it red.

pnpm -F tests test:browser --browser chromium:

  • dropdown-menu — 45 passed
  • context-menu + menubar (same menu.svelte.ts) — 49 passed, 1 skipped

pnpm lint clean, prettier --check clean, changeset included (patch).

Note for consumers

Anyone who worked around this by overriding role after spreading props (that was us) can drop the override once this lands — GroupHeading will render no role at all, so a stale role={undefined} override stays harmless either way.

GroupHeading and Separator both rendered role="group", putting empty groups
into the accessibility tree. Drop the role from GroupHeading (matching
Select/Command, whose group headings set no role) and give Separator its own
separator role (matching the top-level Separator component).
@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e2a1f14

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

This PR includes changesets to release 1 package
Name Type
bits-ui 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

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
bits-ui ✅ Ready (View Log) Visit Preview e2a1f14

@LukianovII

Copy link
Copy Markdown
Author

Test - Chromium is red, but none of the three failures come from this change. Recording the check I did, in case it saves someone the same dig:

failing test evidence it is pre-existing
combobox > should close on outside click same close on outside click failure in #2076, #2084, #2073 (select), #2089, #2088 (combobox + popover) — none of them touch menu.svelte.ts
dropdown-menu > should not scroll to previous dropdown trigger when closing a different dropdown fails identically on #2085, which only renames a Command attribute
context-menu > should allow overriding the pointer events style ran locally on this branch: context-menu 42/42 passed

All three exhaust retry x3 and emit a -4.png screenshot, i.e. they are timing/scroll sensitive rather than assertion failures.

The case this PR adds (dropdown-menu, roles on group/heading/separator) passes on CI — the only dropdown-menu failure in the run is the scroll one above. Happy to rebase if you'd rather see a clean run after the flakes are addressed separately.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu.Separator and Menu.GroupHeading expose role="group"

1 participant