feat(ui): migrate SearchModal + CommandPalette onto Base UI Autocomplete (#520 PR8) - #562
Merged
Conversation
… PR8)
SearchModal's hand-rolled roving-focus result list (activeResultIndex /
moveActiveResult / jumpActiveResult / keyboardSelectionHandledRef /
handleResultKeyDown / data-active) is replaced by Base UI Autocomplete in
activedescendant mode:
- Autocomplete.Root inline + mode="none" + autoHighlight="always" +
filter={null}: the list renders inline in the modal body (no floating
popup), Autocomplete does not re-filter the server-side IPC results, and
the first result is always highlighted so Enter works without an extra
ArrowDown.
- Autocomplete.Input renders the input via the shared InputGroupInput
primitive (render prop); ArrowUp/Down/Enter/Escape keyboard nav is owned
by Autocomplete (floating-ui useListNavigation).
- Autocomplete.List + Autocomplete.Item replace the hand-rolled
<ul role=listbox> + <li><button role=option>; item onClick fires
selectResult for both pointer click and Enter on the highlighted item.
- aria-activedescendant on the input is now managed by Autocomplete.
selectResult navigation (sessionId + turnId, restoreFocus: false), the
debounced IPC search, the inflight ticket guard, the unmount invalidation,
the clear button, the snippet rendering, and all copy/states are unchanged.
The roving-focus kbd-nav interaction (ArrowDown moved focus to the result
button) becomes activedescendant (input keeps focus, active item reflected
via aria-activedescendant). a11y is more standard; the interaction habit
shifts. Home/End now move the input cursor (Base UI ComboboxInput default);
jump-to-first/last result is not bound for now (to be confirmed by manual
testing per PR8 plan).
search-modal-lifecycle-contract: the kbd-nav it-block is rewritten to lock
the Autocomplete shape (Root props + Item onClick + selectResult navigation)
instead of the roving-focus implementation; the focus-policy it-block drops
the activeResultIndex/moveActiveResult assertions; the empty-query it-block
repoints onChange -> onValueChange. CSS .maka-search-modal-result[data-active]
-> [data-highlighted] (Autocomplete item highlighted state).
Verification: typecheck clean, @maka/desktop 2076/2076, @maka/ui 43/43,
sidebar-search-modal-open screenshot AE=2655 (fuzz 5%, RMSE 0.0006) vs main.
CommandPalette's hand-rolled activedescendant result list (highlight
state + onInputKeyDown + reset useEffect + <div role=listbox> +
<Button role=option data-active>) is replaced by Base UI Autocomplete:
- Autocomplete.Root inline + mode="none" + autoHighlight="always" +
filter={null}: the list renders inline in the modal body, Autocomplete
does not re-filter the palette's own fuzzy + content-search combined
list, and the first command is always highlighted so Enter works
without an extra ArrowDown.
- Autocomplete.Input renders the input via the shared InputGroupInput
primitive (render prop); ArrowUp/Down/Enter/Escape keyboard nav is
owned by Autocomplete. aria-controls + aria-activedescendant are
managed by Autocomplete — no manual wiring.
- Autocomplete.List + Autocomplete.Group + Autocomplete.GroupLabel +
Autocomplete.Item replace the hand-rolled <div role=listbox> +
<div group> + <Button role=option>. Autocomplete.Item fires onClick
for both pointer click and Enter on the highlighted item, so commit()
covers both paths.
- The CornerDownLeft cursor hint is now CSS-driven
(.maka-palette-cursor visibility via [data-highlighted]) instead of
the JS `!cmd.hint \&\& active` conditional, since the hand-rolled
highlight state is gone.
commit() (commitPendingRef + committedCommandId + await run + finally
close), the fuzzy filter, useThreadSearch content-search, grouped
rendering, and all copy/states are unchanged.
The kbd-nav interaction stays activedescendant (input keeps focus,
active item reflected via aria-activedescendant) — same mode as before,
now owned by Autocomplete. Home/End now move the input cursor (Base UI
ComboboxInput default); jump-to-first/last command is not bound for now
(to be confirmed by manual testing per PR8 plan).
Contracts: command-palette-a11y-copy-contract #1 (listbox) rewritten to
lock the Autocomplete shape; #2 import regex drops Button (no longer
used); #3 CSS data-active -> data-highlighted; #6 commit-gate block
boundary is commit() (onInputKeyDown gone); #7 highlight reset is now
autoHighlight="always" (no hand-rolled state). renderer-utility-primitives
row assert repoints <Button role=option> -> <Autocomplete.Item>. CSS
.maka-palette-item[data-active] -> [data-highlighted] + .maka-palette-cursor.
Verification: typecheck clean, @maka/desktop 2076/2076, @maka/ui 43/43,
command-palette-open screenshot AE=7250 (fuzz 5%, RMSE 0.0017) vs main.
P1 (blocker): both Autocomplete.Root used `inline` without `open`. Per
Base UI docs, `inline` requires `open` so the list is treated as visible:
"Specify open unconditionally in conjunction with this prop so the list is
considered visible: <Autocomplete.Root inline open>". Without `open`,
defaultOpen=false -> the input is not data-popup-open and keyboard nav /
activedescendant do not work. Add `open` to SearchModal + CommandPalette.
P2-a: object items (<Autocomplete.Item value={result/cmd}>) had no
itemToStringValue, and onValueChange did not filter item-press. Add
itemToStringValue (result.title / cmd.label) so item-press never writes
[object Object] into the query, and skip onValueChange when
details.reason === 'item-press'. In inline mode selectionMode='none' + no
Popup means shouldFillInput is currently false (popupRef.current null), so
this is defensive — but correct regardless of future Popup changes.
P2-b: contract tests now lock open + itemToStringValue + onValueChange
item-press-filter on Autocomplete.Root (catches P1/P2-a regressions), plus
the existing inline/mode/autoHighlight/List/Item shape.
Verified via CDP probe (command-palette-open fixture): input has
data-popup-open + aria-controls + aria-activedescendant; first item
data-highlighted (autoHighlight); ArrowDown 0->1->2, ArrowUp 2->1.
typecheck clean, @maka/ui 43/43, @maka/desktop 2076/2076.
command-palette-open AE=48000 (RMSE 0.011) vs main — larger than pre-fix
AE=7250 because open now correctly renders the first-item highlight bg +
input data-popup-open state (the bug state hid these). search-modal-open
AE=2655 (RMSE 0.0006, unchanged).
P2-c decision: accept Base UI ComboboxInput's default — Home/End move the input cursor, not the highlight. The old roving-focus jumpActiveResult (SearchModal) / hand-rolled onInputKeyDown highlight jump (CommandPalette) must not return. Lock via doesNotMatch on jumpActive( / onInputKeyDown.
…eview P2)
P2-1 (keepHighlight): both Roots had autoHighlight="always" but no
keepHighlight. keepHighlight=false (default) sets resetOnPointerLeave=true
(AriaCombobox.js:855), so pointer leave clears activeIndex, then the
autoHighlight="always" effect (line 678-682) re-highlights the first item
-> hover item[2] -> leave -> Enter ran the first item, not the hovered one.
Add keepHighlight so pointer leave preserves the hovered item.
P2-2 (empty-state): CommandPalette rendered a standalone <div> for empty
and <Autocomplete.List> only when non-empty, so the input lost its listbox
reference with no matches. Unify on Autocomplete.List always, with the Empty
primitive inside. Autocomplete.Empty is not used: filter={null} + mode="none"
keeps filteredItems non-empty (the palette's fuzzy filter is external), so
Autocomplete.Empty would never trigger.
Contracts lock keepHighlight on both Roots + empty-state-must-not-use-
standalone-div.
keepHighlight verified by source (resetOnPointerLeave = !keepHighlight) +
Base UI docs. CDP hover-leave probe could not reliably trigger floating-ui
useListNavigation's hover highlight (synthesized pointermove does not fire
its hover detection), so pointer-leave preservation is source-guaranteed,
not probe-verified. typecheck clean, 2076/2076, screenshots unchanged
(command-palette-open AE=48000, search-modal-open AE=2655 vs main — same as
prior commit, keepHighlight/empty-state don't affect non-empty visuals).
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
Migrate the two hand-rolled search/command result lists onto Base UI Autocomplete (activedescendant mode), converging the last hand-rolled listbox surfaces in the app onto the Base UI primitive.
SearchModal(packages/ui/src/search-modal.tsx): the roving-focus result list (activeResultIndex / moveActiveResult / jumpActiveResult / keyboardSelectionHandledRef / handleResultKeyDown / data-active) is replaced byAutocomplete.Root inline mode="none" autoHighlight="always" filter={null}+Autocomplete.Input(renderInputGroupInput) +Autocomplete.List+Autocomplete.Item.CommandPalette(apps/desktop/src/renderer/command-palette.tsx): the hand-rolled activedescendant list (highlight state + onInputKeyDown + reset useEffect +<div role=listbox>+<Button role=option data-active>) is replaced by the same Autocomplete shape, usingAutocomplete.Group+Autocomplete.GroupLabelfor the grouped command sections.Why
Closes #520 (item: combobox/search-palette convergence onto Base UI). This is the last hand-rolled listbox surface in the app; PR7 already moved the Dialog shells onto Base UI Dialog, and PR9/PR10 converged card/badge/input primitives. PR8 closes the loop on the in-modal search/command lists.
Scope
Changed:
packages/ui/src/search-modal.tsx— rewritten onto Autocomplete; roving-focus machinery deleted.apps/desktop/src/renderer/command-palette.tsx— rewritten onto Autocomplete; highlight state + onInputKeyDown + reset useEffect deleted;Buttonimport dropped (rows areAutocomplete.Item).apps/desktop/src/renderer/styles/sidebar.css—.maka-search-modal-result[data-active="true"]→[data-highlighted];:focus-visiblerow rule dropped (input keeps focus in activedescendant mode).apps/desktop/src/renderer/styles/chat-header.css—.maka-palette-item[data-active="true"]→[data-highlighted](item + icon);.maka-palette-cursorvisibility driven by[data-highlighted](replaces the JS!cmd.hint && activeconditional for the CornerDownLeft cursor hint).search-modal-lifecycle-contract.test.ts— kbd-nav it-block → Autocomplete shape; focus-policy it-block drops activeResultIndex; empty-query it-block repointsonChange→onValueChange.command-palette-a11y-copy-contract.test.ts— listbox it-block → Autocomplete shape; import regex drops Button; CSS data-active → data-highlighted; commit-gate block boundary iscommit()(onInputKeyDown gone); highlight-reset it-block is nowautoHighlight="always".renderer-utility-primitives-contract.test.ts— row assert repoints<Button role=option>→<Autocomplete.Item>; adds Autocomplete import assertion.Not included:
mode="none" + filter={null}preserves the palette's own filtering).Verification
npm run typecheck— clean.npm run -w @maka/ui test— 43/43 pass.npm run -w @maka/desktop test— 2076/2076 pass.sidebar-search-modal-open: AE=2655, RMSE 0.0006 (seed has no query → placeholder state; diff is anti-alias-level from the render-prop input).command-palette-open: AE=7250, RMSE 0.0017 (diff from<div role=option>vs<button>default + data-highlighted bg on the first item).User-facing impact
Keyboard navigation in both modals shifts from roving-focus / hand-rolled activedescendant to Base UI Autocomplete activedescendant:
aria-activedescendant(managed by Autocomplete). Previously SearchModal moved focus to the result button on ArrowDown.useListNavigation).autoHighlight="always": the first result is always highlighted, so Enter on a fresh result set activates the top item without an extra ArrowDown.a11y is more standard (listbox/option/activedescendant owned by the primitive). No CHANGELOG/docs/migration needed (in-app UI, no persisted state).
Reviewer notes
Review follow-up (keepHighlight + empty-state, commit
81743dce):keepHighlight=false(default) setsresetOnPointerLeave=true(AriaCombobox.js:855), so pointer leave clearsactiveIndex, thenautoHighlight="always"(line 678-682) re-highlights the first item → hover item[2] → leave → Enter ran the first item.keepHighlightmakes pointer leave preserve the hovered item. Verified by source + Base UI docs; CDP hover-leave probe could not reliably trigger floating-uiuseListNavigation's hover highlight (synthesized pointermove does not fire its hover detection), so pointer-leave preservation is source-guaranteed rather than probe-verified — the manual checklist covers it.Autocomplete.List. Previously a standalone<div>for empty +<Autocomplete.List>only when non-empty → input lost its listbox reference with no matches. NowAutocomplete.Listalways renders, with theEmptyprimitive inside.Autocomplete.Emptyis not used:filter={null}+mode="none"keepsfilteredItemsnon-empty (the palette's fuzzy filter is external), soAutocomplete.Emptywould never trigger.showEmpty→<p>outside List) has the same shape and was not in this review's scope — flagging for awareness; can unify in a follow-up if wanted.Review fixes (P1 + P2-a + P2-b + P2-c, commit
aec3258f):opento both Autocomplete.Root. Per Base UI docs,inlinerequiresopenso the list is treated as visible. Without it,defaultOpen=false→ input notdata-popup-open, keyboard nav / activedescendant break. Verified via CDP probe: input hasdata-popup-open+aria-controls+aria-activedescendant; first itemdata-highlighted; ArrowDown 0→1→2, ArrowUp 2→1.itemToStringValue+ onValueChange item-press guard. Object items (result/cmd) now serialize to title/label;onValueChangeskipsdetails.reason === 'item-press'so selection never writes the object back into the query. Defensive (inline mode'sshouldFillInputis currently false becausepopupRef.currentis null), but correct regardless of future Popup changes.open+itemToStringValue+ onValueChange item-press filter (catches P1/P2-a regressions), plus the existing inline/mode/autoHighlight/List/Item shape. Keyboard event simulation needs happy-dom (not in maka's test infra — its DOM tests use a hand-rolled FakeElement without an event system), so keyboard nav is covered by the CDP probe + the manual checklist below rather than automated tests.jumpActive/onInputKeyDown).Manual checklist (please confirm by hand, both SearchModal and CommandPalette):
[object Object]or the item label.Four commits: SearchModal (
598b170a), CommandPalette (63e93427), review fixes (aec3258f), keepHighlight + empty-state (81743dce). Each independently revertable.Checklist