fix: group menu bar recent items and disable empty submenus - #160
Merged
Conversation
Radix renders MenuLabel as an unroled div, so the two headings named nothing on their own and the submenu read as one undifferentiated run of paths. Wrapping each block in MenubarGroup with aria-labelledby gives the paths a section boundary, which matters most when the same directory appears in both lists.
An enabled trigger over an all-disabled command set opened a menu Radix focused with nothing in it, which arrow keys could not move through. The availability rule already said inactive commands are disabled rather than hidden; extending it to the trigger keeps the two consistent. Both derived-state submenus are covered: `Sort articles by` is the radio case, disabled with no folder context open. `Line ending` and `Open recent` are deliberately not, because each holds a command that stays available on its own.
`aria-label` on a generic element is not exposed, so the name was already dropped. It is deleted rather than relocated: Radix supplies role="menubar" on the root, and naming that "Menu bar" would only repeat the role for the one menubar in the app.
8 tasks
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.
Summary
Three structural problems in the command menu bar, all in the same markup.
MenuLabelas an unroleddiv, and nothing wrapped the items beneath it, soRecent filesandRecent foldersnamed nothing and the submenu read as one undifferentiated run of paths. Each block is now aMenubarGroupwitharia-labelledbypointing at its label, so every path is announced inside its section. That boundary matters most when the same directory appears in both lists — a folder that was opened, plus a file inside it.role="menu"container, and arrow keys had nothing to move through. Triggers now derivedisabledfrom the commands behind them.aria-label="Menu bar"sat on a rolelessdiv, where it is never exposed. It is deleted rather than relocated: Radix already suppliesrole="menubar"on the root, and naming it "Menu bar" would repeat the role for the only menubar in the app.RadioSubmenuis covered alongsideCommandSubmenubecause it has a real case, not for symmetry: all threeview.sort.*commands are disabled when no folder context is open, soSort articles byhad the same empty-submenu shape asTable.LineEndingSubmenuandRecentItemsSubmenuare deliberately excluded — each holds a command that stays available on its own (Insert final newline on save,Clear recent items), so neither can reach the all-disabled state.MenubarSubTriggerhad nodata-disabledstyling, because nothing disabled it before. It now takes the samepointer-events-noneandopacity-50treatmentMenubarItemalready carried.Related Issue
Closes #119
Verification
Automated (
pnpm check:frontend, Windows 11, 922 frontend tests across 92 files, exit 0):src/components/layout/CommandMenuBar.test.tsxgains five tests. Grouping asserts each path resolveswithinits ownrole="group". The submenu rule is covered from four directions: all-disabled (Tablecarriesdata-disabledandaria-disabled, and clicking it does not open), partially disabled (Tablestays enabled and opens, with the unavailable items disabled inside), the radio case (Sort articles bydisabled,Appearancenot), and the always-available case (Line endingstays enabled whileCopy asdoes not).CommandMenuBar.tsxfails the grouping test and three of the four submenu tests. The fourth — a partially disabled trigger stays enabled — passes either way by design; it is the guard against over-disabling, not a regression test for this fix.Manual, on Windows 11 through
pnpm tauri dev, with Narrator. jsdom applies no Tailwind and has no accessibility tree, so the dimmed trigger and the spoken group boundary are only observable here:Format > TableandView > Sort articles byrender dimmed and do not open when every command behind them is unavailable.File > Open recentannounces each path withinRecent filesorRecent foldersrather than as one continuous run.Escapehandling, and focus return to the trigger on close are unchanged.Not verified: platforms other than Windows, and a bundled build.
Notes
docs/reference.mdextends the availability rule rather than adding a new one. The rule already said inactive commands are disabled rather than hidden; a trigger that opens onto nothing but disabled commands was the same situation one level up.DropdownMenu.tsxhas the same undecorated sub-trigger, left alone because nothing disables it there.