Repository navigation
feat(web): tidy workspace menus, dock and sidebar controls - #256
Conversation
WebKit does not focus a button on mouse click, so Menu read <body> from document.activeElement and opened the popup at the top-left corner. Anchor to the click's currentTarget, keeping focus as the fallback for bare open(), and re-place the open menu on resize and scroll. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Absolutely positioned descendants such as .sr-only labels escaped the scrolling frame and extended the document, so the shell could scroll up and expose the page background below it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kinqsradio
left a comment
There was a problem hiding this comment.
Reviewed #256 against feat/web-rebuild (cadf0db6). The consolidation is good: one account menu, a labelled collapse control, consistent menu rows and a cleaner dock header. The manual checks at 1440/1200/700/375 are useful.
This cannot merge as-is: it conflicts with #254 in web/src/ui/kit.tsx. I verified with git merge-tree; each branch merges cleanly into the base alone, but #254 + #256 produce a content conflict. Rebase #256 after #254 and keep both sides:
- #254's event-based
open(event)and itsplace()+ resize/scroll listeners; - #256's
side,align,fitTriggerandbutton:not(:disabled)initial focus.
In the merged place(), re-apply the fitTrigger width on resize too, not just left/top, and include side/align/fitTrigger in the layout-effect dependencies. Merge order matters beyond the text conflict: the new account/tab menus here (and #255's new menus) pass open directly to onClick, so on WebKit they rely on #254 to avoid anchoring at the top-left.
After the rebase, the inline comments cover the menu arrow-key handling and the rail account button's accessible name. tsc --noEmit -p web/tsconfig.json, Prettier on the changed files and npm run build -w web pass locally. No CI checks are reported, and I did not run a keyboard/screen-reader pass.
| menu.style.top = `${below ? at.bottom + 4 : Math.max(8, at.top - size.height - 4)}px`; | ||
| menu.querySelector("button")?.focus(); | ||
| }, [opener]); | ||
| menu.querySelector<HTMLElement>("button:not(:disabled)")?.focus(); |
There was a problem hiding this comment.
Initial focus now skips disabled buttons, but the arrow handler at line 341 still builds its item list with querySelectorAll("button"). With the new disabled rows, focus() on a disabled button no-ops and the next Arrow press tries the same row again, so navigation can get stuck. Filter to button:not(:disabled) in the arrow handler too.
| const size = menu.getBoundingClientRect(); | ||
| const left = Math.max(8, Math.min(at.left, innerWidth - size.width - 8)); | ||
| const below = at.bottom + 4 + size.height <= innerHeight - 8; | ||
| const wanted = align === "end" ? at.right - size.width : at.left; |
There was a problem hiding this comment.
This layout effect overlaps #254's changes to the same Menu function. After rebasing, keep #254's place() plus its resize/scroll listeners and call the combined function here. Also re-apply the fitTrigger width inside place(), not just left/top, so resizing the sidebar does not leave the account menu at its old width.
| fitTrigger={where === "sidebar" && !size.rail} | ||
| trigger={(open, expanded) => ( | ||
| <button | ||
| className={`account account-${where}`} |
There was a problem hiding this comment.
In the rail, .shell-nav-text is display: none and the avatar is aria-hidden, so the sidebar account button has no accessible name — aria-label is only set for the topbar variant. Set the label for the sidebar/rail case too. Consider adding aria-haspopup="menu" to this trigger for consistency with the other menu buttons.
- Workspace, tab and account menus use grouped menu rows with icons instead of pill buttons; tab actions use the shared Menu (Escape, outside click, arrow keys) and drop the Done button. - The shared Menu can open above, align to the trigger's end edge and match the trigger's width. - The dock header is one row with icon actions; under 760px the dock is a right-hand sheet instead of a half-height strip. - The account menu opens above the sidebar trigger at sidebar width and moves to the top bar when the sidebar becomes a drawer (<=800px). - Arrange navigation moves into the account menu, the Icon rail button becomes a side-panel toggle beside the logo, and Arrange rows show each destination's icon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ef40807 to
e8e6562
Compare
|
Rebased onto #254 ( Also fixed: arrow navigation and initial focus skip disabled buttons; the rail account button keeps its accessible name and reports Checks: Prettier and |
See CONTRIBUTING before starting engineering work.
Change
Makes the web rebuild's pop-ups consistent and keeps side panels on the right.
Stacked on #254 (
99818e49). This branch contains #254's commits; merge #254 first.··· Workspace): left-aligned rows with icons, grouped under Layout and Dock, end-aligned to the button. "Reopen closed tab" is a normal row, disabled when nothing is closed. Previously the items were<Button>pills, whose.btnstyles overrode.menurows.···on a tab): now uses the sharedMenu(closes on pick, Escape or outside click; arrow keys move), grouped as Pin/Duplicate · Move · Close, with icons. The Done button and the old.workspace-tab-actionsstyles are removed.Menu(ui/kit.tsx): newside,alignandfitTriggeroptions; focuses the first enabled item; adds.menu-label, separators and disabled/checked row styles.Checks and limits
tsc --noEmitinweb/and Prettier on the changed files: pass.Follow-up
e8e6562a(3 Nov 2026): rebased onto #254 (99818e49) and resolved thekit.tsxconflict. The merge keeps event-based anchoring and scroll/resize placement plusside/align/fitTrigger, reapplying trigger width on resize; arrow navigation and initial focus skip disabled items; the rail account trigger keeps its accessible name and addsaria-haspopup="menu". Prettier andvite buildpass;tsc --noEmitpasses with the ES2023 array lib. The stocknpm run buildstill stops at the pre-existingArray.findLast/ES2022 error inlibrary/page/Source.tsx. No browser pass was run after the follow-up.Workspace
repo/, and read the relevant project guides.This contributor confirmation is not an automated claim that a host loaded policies.
Agent disclosure
claude-opus-5-5), default effort.🤖 Generated with Claude Code
Follow-up fixes (3 Nov 2026)
git,gh,npm,tsc, Prettier, Vite.