feat(menu): allow overriding the popover role - #565
Merged
Conversation
RuiMenu hardcoded role="menu" on its teleported popover, which forces the ARIA model on every consumer. That is wrong when the content is a selection list: a listbox may not nest inside a menu, so an input opening it cannot be a combobox. Add a `role` prop (menu | listbox | tree | grid | dialog) defaulting to "menu", and derive the activator's aria-haspopup from it so the two agree. The default keeps emitting aria-haspopup="true", which is equivalent to "menu", so nothing changes for existing callers. Closes rotki#564
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
=======================================
Coverage 87.04% 87.04%
=======================================
Files 159 159
Lines 6306 6307 +1
Branches 1938 1939 +1
=======================================
+ Hits 5489 5490 +1
Misses 817 817 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #564
Problem
RuiMenuhardcodedrole="menu"on its teleported popover, with no attrs spread and no prop to change it.RuiMenuis the popover primitive for things that are not menus, and the container role forces the ARIA model: alistboxcannot nest inside amenu, so an input that opens one cannot legitimately be acombobox.Change
roleprop onRuiMenu:'menu' | 'listbox' | 'tree' | 'grid' | 'dialog', defaulting to'menu'.aria-haspopupis derived from it so the two agree. For the default it still emitsaria-haspopup="true", which is equivalent to"menu", so existing callers and their tests see no change.RuiMenuRoleis exported from the components entry.aria-haspopup.Notes
The library's own selection components (
RuiAutoComplete,RuiMenuSelect, ...) still use the default menu role. Moving them to the listbox/combobox model is a larger change (option roles,aria-activedescendant,aria-selected) and is left out of this PR.Testing
pnpm run test:run(1346 passing)pnpm run typecheckpnpm run lint(no new warnings)