refactor(ui): unify the workspace picker onto the composer's ghost-menu family - #2287
Merged
Merged
Conversation
…u family The composer footer mixed two component families for one toolbar role after #2230 moved the model and thinking pickers onto ghost-button DropdownMenus: +/permission/model/thinking were ghost buttons, while the project picker stayed an Astryx Selector field disguised by ~60 lines of product CSS that cleared field chrome state by state. That is why the chip never matched its neighbours — Selector's own label weight and ink against the Button's, no tooltip (Selector has no such slot, so the current git branch rode in the accessible name alone), and a focus fill with no accent ring (#2230 kept the quiet Selector overlay until it got its own migration; this is that migration). The picker is now a chevronless ghost-button DropdownMenu like the model chip beside it, so resting, hover, focus, disabled and tooltip chrome all derive from one Button: - Trigger: FolderOpen icon + current project (or 无项目), tooltip with the current branch (`选择项目 · <branch>`), accessible name unchanged. - Rows: project list with the current value's plain check and the relink status on unavailable projects, then 添加项目 / 无项目 pinned to the menu's bottom edge as one role=group box (the CSS pinning contract moves from [role=listbox] to [role=menu]). - Search dies with the Selector, matching the model menu's precedent: the panel keeps its 300px scroll and Astryx first-character typeahead. - Pending (switch in flight) locks the trigger with a spinner and every row, like the model switcher. Also removes the now-dead Selector overlay rules and the searchPlaceholder copy; the palette seam comment now covers popovers, not just value controls.
The NewChatComposer story covers the picker's resting state and its menu; nothing covered `pending: true` — the trigger locks with a spinner and every row disables, matching the model switcher's mid-switch treatment. Add a variant that passes `workspacePicker.pending`.
The pinned actions were sticky inside the menu panel, which is itself the scroller: when the wheel reached the catalogue's end, overscroll chained into the page behind the popover and dragged the whole inline-rendered panel — pinned actions included — with it. Restructure instead of re-pinning: the catalogue renders in its own scroll region (`.maka-workspace-picker-scroll`, the pre-Selector design from before #2217) and the two actions sit after it in normal flow, physically outside the scroller. The panel lifts Astryx's 300px cap (max-height: none, overflow: visible) so it can never scroll on its own; the region owns the height budget (224px of catalogue + the actions ≈ the old 300px ceiling) and `overscroll-behavior: contain` stops the wheel from chaining past its ends. The sticky rules, the group backdrop, and the padding restoration all die with the construction they propped up; the divider between catalogue and actions stays on the group's top border, dropped on first run when the group is the menu's first child. The CSS pinning contract test is rewritten to pin the new construction: exactly one scroller under the menu (the catalogue region, with the height budget), no sticky anywhere, no backdrop painted on the group or rows, and the window cap intact. Verified in Storybook: the actions' rect is constant while the region scrolls and after a wheel over it; keyboard roving reaches the actions (they were unreachable while the menu accidentally rendered two copies of the catalogue during this refactor — the single-copy tree roves maka-agent → … → 添加项目 → 无项目 and stops).
Follow-up from two external reviews of #2287: the behaviors that mattered most were verified manually, not by tests. Pin them. - SSR structure tests: the catalogue renders exactly once, inside its own scroll region with the actions after it (the mid-PR duplicate-catalogue regression that made keyboard roving unreachable would now go red); the unavailable project's relink row; the current-value check on the selected project and on the no-project row; first run renders no scroll region at all. - SSR lock test mirroring chat-model-switcher: pending locks the trigger (aria-disabled + aria-busy) and every menu row — an aria-disabled trigger still opens on ArrowDown, so the lock must ride on the items. Reverse at-rest assertion included. - CSS contract: the panel "may lift" the 300px cap is now "must lift" (max-height: none + overflow: visible are required — Astryx's cap comes with overflow-y: auto, which scrolls the actions with the panel again), the scroller must declare overscroll-behavior: contain (the chaining fix), and the :not(:first-child) divider rule is pinned. Mutation check: dropping overscroll-behavior: contain now fails the contract (was green before). Also trims what the design review found: the stale header doc (panel "keeps its 300px scroll" — it no longer scrolls at all; typeahead is now described accurately for CJK labels), Check aligned to the family's size 14, the dead .maka-workspace-picker-actions class, and the ::-webkit-scrollbar block that standard scrollbar-color/width override in Chromium 121+.
Contributor
Author
|
External review follow-up (2× deepseek-v4-flash): design review found no P0/P1/P2 — agreed no shared QuietFooterMenu abstraction, search→typeahead tradeoff defensible; test review found the load-bearing CSS and the pending lock unpinned (P2-1/P2-2/P2-3, mutation-verified). All accepted findings fixed in c658415 (structure/lock SSR tests + contract must-lift/overscroll/divider assertions + doc/trim cleanups), mutation-checked: the contract now goes red when |
Astro-Han
marked this pull request as ready for review
August 5, 2026 19:40
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.
Background
After #2230 moved the model and thinking pickers onto ghost-button
DropdownMenus, the project picker became the last AstryxSelector(a form-field primitive) left in the composer footer — the one toolbar row where every other control (+, permission, model, thinking) is a ghost button. #2230's commit message explicitly left this as a follow-up: "The workspace picker keeps its quiet Selector overlay until it gets its own migration."Every symptom reported maps to that family mismatch:
Selectorhas no tooltip slot (refactor(composer): drop the branch picker and rebuild the project picker on Selector #2217 recorded this loss), so the current git branch only rode in the accessible name.box-shadow: none), while ghost buttons get the accent focus-visible ring.Change
WorkspacePickermigrates fromSelectorto the same chevronless ghost-buttonDropdownMenuas the model chip beside it, so resting, hover, focus, disabled and tooltip chrome all derive from one AstryxButton:选择项目 · <branch>, branch stays read-only context); accessible name unchanged..maka-workspace-picker-scroll, 224px budget,overscroll-behavior: contain); the menu panel lifts Astryx's 300px cap (max-height: none; overflow: visible) so it never scrolls itself. This replaces the sticky-pinned footer from the first iteration of this PR, which moved with the panel when overscroll chained into the page behind the popover.Also removes the now-dead Selector overlay rules, the
searchPlaceholdercopy, a dead::-webkit-scrollbarblock (standardscrollbar-color/scrollbar-widthwin in Chromium 121+) and a dead class hook; restores thechoose/chooseTitlecopy; updates the palette-seam and header docs.Storybook
The existing
NewChatComposerstory already renders the picker (draft state); this PR addsNewChatComposerProjectPendingfor the uncoveredpending: truestate.Testing
Two external reviews (deepseek-v4-flash, design + test angles) found no P0/P1/P2 in the design, but flagged that the two behaviors that mattered most were verified manually, not by tests. Both gaps are now closed:
SSR structure tests (composer-workspace-picker.test.tsx, +5 cases): the catalogue renders exactly once in its own region with the actions after it (the mid-PR duplicate-catalogue regression that broke keyboard roving would now go red); relink row; current-value check placement (selected project / no-project row); first run renders no scroll region; pending locks the trigger (
aria-disabled+aria-busy) and every row, with an at-rest reverse assertion — mirroring chat-model-switcher's lock pin.CSS contract extensions (workspace-picker-menu-contract.test.ts, +2 cases): the panel must lift the cap (
max-height: none+overflow: visible— Astryx's cap comes withoverflow-y: auto), the scroller must declareoverscroll-behavior: contain, and the:not(:first-child)divider rule is pinned. Mutation-checked: droppingoverscroll-behavior: containnow fails the contract (was green before).@maka/ui: 419 tests pass; contract test 5/5; check-dead-css 13/13; biome format/lint clean.Live Storybook checks (Playwright + computed styles): trigger and model chip report identical computed styles (color
oklch(0.17 0.005 286), weight 500, 14px, radius 16px); keyboard focus shows the accent ring; the actions' rect is constant while the catalogue scrolls and after a wheel over it; keyboard roving reaches every row and stops (no wrap); tooltip renders "选择项目 · opencode/storybook-surface-coverage"; pending story showsaria-disabled+aria-busy+ spinner.