From 7ca764f91766d2d158f3bf14f2bc27d75c4e6f3a Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 15:09:01 -0700 Subject: [PATCH] canvas/SearchDialog: auto-highlight first match + semantic placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small UIUX fixes for Cmd+K search. 1. Auto-highlight the first match while the user types. Before, Enter on a non-empty query was a no-op — focusedIndex stayed at -1 until the user pressed ↓. Standard search-palette behavior is to highlight the top result so Enter just works. Empty query keeps -1 (opening the dialog shows ALL workspaces; arbitrarily pinning one looks wrong). 2. placeholder-zinc-400 → placeholder-ink-soft. The hardcoded zinc broke the semantic-token pattern other inputs use; placeholder now flips with theme correctly. (Also reordered focus:outline-none ahead of the focus-visible variants — cosmetic, more idiomatic.) Tests: replaced the "resets to -1" test with two new ones — auto- highlight on a matching query (Enter selects without ArrowDown), and no-results query stays a no-op. Full suite 1220/1220. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/SearchDialog.tsx | 19 ++++++++---- .../__tests__/SearchDialog.keyboard.test.tsx | 31 +++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/canvas/src/components/SearchDialog.tsx b/canvas/src/components/SearchDialog.tsx index 110c031f..4fa2e700 100644 --- a/canvas/src/components/SearchDialog.tsx +++ b/canvas/src/components/SearchDialog.tsx @@ -36,11 +36,6 @@ export function SearchDialog() { } }, [open]); - // Reset focused index when query changes - useEffect(() => { - setFocusedIndex(-1); - }, [query]); - const filtered = nodes.filter((n) => { if (!query) return true; const q = query.toLowerCase(); @@ -51,6 +46,18 @@ export function SearchDialog() { ); }); + // Auto-highlight the first match while the user is typing, so Enter + // selects something instead of being a no-op. With an empty query we + // keep -1 so opening the dialog (which shows ALL workspaces) doesn't + // visually pin one row arbitrarily — only commit a highlight once the + // user has narrowed the list. + useEffect(() => { + setFocusedIndex(query && filtered.length > 0 ? 0 : -1); + // Re-running on filtered.length keeps the highlight pinned to the + // first row while the result set shrinks/grows; the effect handler + // above already short-circuits to -1 when results disappear. + }, [query, filtered.length]); + const handleSelect = useCallback( (nodeId: string) => { selectNode(nodeId); @@ -113,7 +120,7 @@ export function SearchDialog() { onChange={(e) => setQuery(e.target.value)} onKeyDown={handleInputKeyDown} placeholder="Search workspaces..." - className="flex-1 bg-transparent text-sm text-ink placeholder-zinc-400 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus:outline-none rounded" + className="flex-1 bg-transparent text-sm text-ink placeholder-ink-soft focus:outline-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent rounded" /> ESC diff --git a/canvas/src/components/__tests__/SearchDialog.keyboard.test.tsx b/canvas/src/components/__tests__/SearchDialog.keyboard.test.tsx index 45e1bcfc..2eefec48 100644 --- a/canvas/src/components/__tests__/SearchDialog.keyboard.test.tsx +++ b/canvas/src/components/__tests__/SearchDialog.keyboard.test.tsx @@ -155,18 +155,31 @@ describe("SearchDialog — keyboard accessibility", () => { expect(selectNode).not.toHaveBeenCalled(); }); - it("typing a new query resets focusedIndex to -1", () => { + it("typing a query that matches auto-highlights the first result", () => { + // Replaces the older "resets to -1" assertion. New behavior: a query + // with at least one match pins the highlight to row 0 so Enter picks + // a result instead of being a no-op. Empty-query case is covered by + // "Enter at focusedIndex=-1 does not select anything" above. + render(); + const input = screen.getByRole("combobox"); + fireEvent.change(input, { target: { value: "Alpha" } }); + const options = screen.getAllByRole("option"); + expect(options[0].getAttribute("aria-selected")).toBe("true"); + // Enter on the auto-highlighted match should select it without + // needing a manual ArrowDown first. + fireEvent.keyDown(input, { key: "Enter" }); + expect(selectNode).toHaveBeenCalledWith("ws-1"); + }); + + it("typing a query that matches NOTHING resets focusedIndex to -1", () => { render(); const input = screen.getByRole("combobox"); fireEvent.keyDown(input, { key: "ArrowDown" }); // focusedIndex → 0 - // Verify selection before reset - expect(screen.getAllByRole("option")[0].getAttribute("aria-selected")).toBe("true"); - // Change query — triggers the useEffect that resets focusedIndex - fireEvent.change(input, { target: { value: "Alpha" } }); - // After reset all options must have aria-selected="false" - screen.getAllByRole("option").forEach((opt) => { - expect(opt.getAttribute("aria-selected")).toBe("false"); - }); + fireEvent.change(input, { target: { value: "zzz-no-match" } }); + // No options remain, so nothing to assert on aria-selected directly — + // the empty-state message takes over. But Enter should be a no-op. + fireEvent.keyDown(input, { key: "Enter" }); + expect(selectNode).not.toHaveBeenCalled(); }); it("aria-activedescendant matches the focused option's id", () => {