From bdb9fabfd0a47089f4c6bc7e75f4c3fc394d2a2e Mon Sep 17 00:00:00 2001 From: lucaronin Date: Wed, 27 May 2026 11:13:45 +0200 Subject: [PATCH] fix: stabilize global search keyboard navigation --- src/components/SearchPanel.test.tsx | 128 +++++++++++++++- src/components/SearchPanel.tsx | 147 +++++++++++++++---- tests/smoke/keyboard-command-routing.spec.ts | 58 ++++++++ 3 files changed, 301 insertions(+), 32 deletions(-) diff --git a/src/components/SearchPanel.test.tsx b/src/components/SearchPanel.test.tsx index 7c8e579e..082bac85 100644 --- a/src/components/SearchPanel.test.tsx +++ b/src/components/SearchPanel.test.tsx @@ -117,7 +117,7 @@ describe('SearchPanel', () => { render( , ) - fireEvent.keyDown(window, { key: 'Escape' }) + fireEvent.keyDown(document, { key: 'Escape' }) expect(onClose).toHaveBeenCalled() }) @@ -217,6 +217,132 @@ describe('SearchPanel', () => { }) }) + it('moves down one result when ArrowDown is pressed in the input', async () => { + const entries: VaultEntry[] = [ + ...MOCK_ENTRIES, + { + ...MOCK_ENTRIES[0], + path: '/vault/topic/search.md', + filename: 'search.md', + title: 'Search Patterns', + }, + ] + mockInvokeFn.mockResolvedValue({ + results: [ + { title: 'Result One', path: '/vault/essay/ai-apis.md', snippet: 'First result', score: 0.9, note_type: null }, + { title: 'Result Two', path: '/vault/event/retreat.md', snippet: 'Second result', score: 0.8, note_type: null }, + { title: 'Result Three', path: '/vault/topic/search.md', snippet: 'Third result', score: 0.7, note_type: null }, + ], + elapsed_ms: 20, + }) + + render( + , + ) + + const input = screen.getByPlaceholderText('Search in all notes...') + fireEvent.change(input, { target: { value: 'test' } }) + + await waitFor(() => { + expect(screen.getByText('Search Patterns')).toBeInTheDocument() + }) + + await act(async () => { + fireEvent.keyDown(input, { key: 'ArrowDown' }) + }) + + await waitFor(() => { + const resultTwo = screen.getByText('Refactoring Retreat').closest('[class*="cursor-pointer"]')! + const resultThree = screen.getByText('Search Patterns').closest('[class*="cursor-pointer"]')! + expect(resultTwo.className).toContain('bg-accent') + expect(resultThree.className).not.toContain('bg-accent') + }) + }) + + it('ignores duplicate native ArrowDown keydown events for one press', async () => { + const entries: VaultEntry[] = [ + ...MOCK_ENTRIES, + { + ...MOCK_ENTRIES[0], + path: '/vault/topic/search.md', + filename: 'search.md', + title: 'Search Patterns', + }, + ] + mockInvokeFn.mockResolvedValue({ + results: [ + { title: 'Result One', path: '/vault/essay/ai-apis.md', snippet: 'First result', score: 0.9, note_type: null }, + { title: 'Result Two', path: '/vault/event/retreat.md', snippet: 'Second result', score: 0.8, note_type: null }, + { title: 'Result Three', path: '/vault/topic/search.md', snippet: 'Third result', score: 0.7, note_type: null }, + ], + elapsed_ms: 20, + }) + + render( + , + ) + + const input = screen.getByPlaceholderText('Search in all notes...') + fireEvent.change(input, { target: { value: 'test' } }) + + await waitFor(() => { + expect(screen.getByText('Search Patterns')).toBeInTheDocument() + }) + + await act(async () => { + fireEvent.keyDown(input, { key: 'ArrowDown' }) + fireEvent.keyDown(input, { key: 'ArrowDown' }) + }) + + await waitFor(() => { + const resultTwo = screen.getByText('Refactoring Retreat').closest('[class*="cursor-pointer"]')! + const resultThree = screen.getByText('Search Patterns').closest('[class*="cursor-pointer"]')! + expect(resultTwo.className).toContain('bg-accent') + expect(resultThree.className).not.toContain('bg-accent') + }) + }) + + it('allows a second ArrowDown press immediately after keyup', async () => { + const entries: VaultEntry[] = [ + ...MOCK_ENTRIES, + { + ...MOCK_ENTRIES[0], + path: '/vault/topic/search.md', + filename: 'search.md', + title: 'Search Patterns', + }, + ] + mockInvokeFn.mockResolvedValue({ + results: [ + { title: 'Result One', path: '/vault/essay/ai-apis.md', snippet: 'First result', score: 0.9, note_type: null }, + { title: 'Result Two', path: '/vault/event/retreat.md', snippet: 'Second result', score: 0.8, note_type: null }, + { title: 'Result Three', path: '/vault/topic/search.md', snippet: 'Third result', score: 0.7, note_type: null }, + ], + elapsed_ms: 20, + }) + + render( + , + ) + + const input = screen.getByPlaceholderText('Search in all notes...') + fireEvent.change(input, { target: { value: 'test' } }) + + await waitFor(() => { + expect(screen.getByText('Search Patterns')).toBeInTheDocument() + }) + + await act(async () => { + fireEvent.keyDown(input, { key: 'ArrowDown' }) + fireEvent.keyUp(input, { key: 'ArrowDown' }) + fireEvent.keyDown(input, { key: 'ArrowDown' }) + }) + + await waitFor(() => { + expect(screen.getByText('Search Patterns').closest('[class*="cursor-pointer"]')!.className).toContain('bg-accent') + }) + }) + it('selects result on Enter and calls onSelectNote', async () => { mockInvokeFn.mockResolvedValue({ results: [ diff --git a/src/components/SearchPanel.tsx b/src/components/SearchPanel.tsx index a3a5bfc4..580ec0e8 100644 --- a/src/components/SearchPanel.tsx +++ b/src/components/SearchPanel.tsx @@ -21,6 +21,23 @@ interface SearchPanelProps { type SearchKeyboardAction = 'close' | 'next' | 'previous' | 'select' +interface SearchKeyboardEvent { + key: string + nativeEvent?: Event + preventDefault: () => void + repeat?: boolean + stopImmediatePropagation?: () => void + stopPropagation?: () => void +} + +interface SearchKeyboardActionContext { + handleSelect: (result: SearchResult) => void + onClose: () => void + resultsRef: React.MutableRefObject + selectedIndexRef: React.MutableRefObject + setSelectedIndex: React.Dispatch> +} + function resolveSearchKeyboardAction(key: string): SearchKeyboardAction | null { switch (key) { case 'Escape': @@ -41,10 +58,89 @@ function nextSearchSelectionIndex( currentIndex: number, resultCount: number, ): number { + if (resultCount <= 0) return 0 if (action === 'next') return Math.min(currentIndex + 1, resultCount - 1) return Math.max(currentIndex - 1, 0) } +function shouldHandleKeydown( + event: SearchKeyboardEvent, + pressedKeys: Set, + handledEvents: WeakSet, +): boolean { + const eventIdentity = resolveSearchKeyboardEventIdentity(event) + if (eventIdentity) { + if (handledEvents.has(eventIdentity)) return false + handledEvents.add(eventIdentity) + } + + if (event.repeat) return true + if (pressedKeys.has(event.key)) return false + + pressedKeys.add(event.key) + return true +} + +function resolveSearchKeyboardEventIdentity(event: SearchKeyboardEvent): Event | null { + if (event.nativeEvent instanceof Event) return event.nativeEvent + if (event instanceof Event) return event + return null +} + +function applySearchSelection( + action: Extract, + resultsRef: React.MutableRefObject, + selectedIndexRef: React.MutableRefObject, + setSelectedIndex: React.Dispatch>, +) { + const nextIndex = nextSearchSelectionIndex(action, selectedIndexRef.current, resultsRef.current.length) + selectedIndexRef.current = nextIndex + setSelectedIndex(nextIndex) +} + +function performSearchKeyboardAction(action: SearchKeyboardAction, context: SearchKeyboardActionContext) { + if (action === 'close') { + context.onClose() + return + } + + if (action === 'select') { + const result = context.resultsRef.current[context.selectedIndexRef.current] + if (result) context.handleSelect(result) + return + } + + applySearchSelection(action, context.resultsRef, context.selectedIndexRef, context.setSelectedIndex) +} + +function useSearchKeyboardDocumentListeners({ + handleKeyDown, + handleKeyUp, + open, + pressedKeysRef, +}: { + handleKeyDown: (event: KeyboardEvent) => void + handleKeyUp: (event: KeyboardEvent) => void + open: boolean + pressedKeysRef: React.MutableRefObject> +}) { + useEffect(() => { + const pressedKeys = pressedKeysRef.current + if (!open) { + pressedKeys.clear() + return + } + + document.addEventListener('keydown', handleKeyDown, true) + document.addEventListener('keyup', handleKeyUp, true) + return () => { + document.removeEventListener('keydown', handleKeyDown, true) + document.removeEventListener('keyup', handleKeyUp, true) + pressedKeys.clear() + } + }, [handleKeyDown, handleKeyUp, open, pressedKeysRef]) +} + function searchVaultPathsForEntries(entries: VaultEntry[], fallbackVaultPath: string): string | string[] { const paths = entries .map((entry) => entry.workspace?.path) @@ -95,33 +191,25 @@ function useSearchKeyboard({ selectedIndexRef: React.MutableRefObject setSelectedIndex: React.Dispatch> }) { - const handleKeyDown = useCallback((e: { key: string; preventDefault: () => void }) => { + const pressedKeysRef = useRef(new Set()) + const handledEventsRef = useRef(new WeakSet()) + const handleKeyDown = useCallback((e: SearchKeyboardEvent) => { const action = resolveSearchKeyboardAction(e.key) if (!action) return e.preventDefault() - if (action === 'close') { - onClose() - return - } + e.stopImmediatePropagation?.() + e.stopPropagation?.() + if (!shouldHandleKeydown(e, pressedKeysRef.current, handledEventsRef.current)) return - if (action === 'select') { - const result = resultsRef.current[selectedIndexRef.current] - if (result) handleSelect(result) - return - } - - setSelectedIndex(i => nextSearchSelectionIndex(action, i, resultsRef.current.length)) + performSearchKeyboardAction(action, { handleSelect, onClose, resultsRef, selectedIndexRef, setSelectedIndex }) }, [handleSelect, onClose, resultsRef, selectedIndexRef, setSelectedIndex]) - useEffect(() => { - if (!open) return - const handleKey = (e: KeyboardEvent) => handleKeyDown(e) - window.addEventListener('keydown', handleKey) - return () => window.removeEventListener('keydown', handleKey) - }, [open, handleKeyDown]) + const handleKeyUp = useCallback((e: { key: string }) => { + if (resolveSearchKeyboardAction(e.key)) pressedKeysRef.current.delete(e.key) + }, []) - return handleKeyDown + useSearchKeyboardDocumentListeners({ handleKeyDown, handleKeyUp, open, pressedKeysRef }) } function useSearchPanelController({ open, vaultPath, entries, onSelectNote, onClose }: SearchPanelProps) { @@ -150,7 +238,7 @@ function useSearchPanelController({ open, vaultPath, entries, onSelectNote, onCl if (open) setTimeout(() => inputRef.current?.focus(), 50) }, [open]) - const handleKeyDown = useSearchKeyboard({ + useSearchKeyboard({ open, onClose, handleSelect, @@ -162,7 +250,6 @@ function useSearchPanelController({ open, vaultPath, entries, onSelectNote, onCl return { elapsedMs, - handleKeyDown, handleSelect, inputRef, listRef, @@ -188,7 +275,6 @@ export function SearchPanel({ const { elapsedMs, entryLookup, - handleKeyDown, handleSelect, inputRef, listRef, @@ -236,7 +322,6 @@ export function SearchPanel({ query={query} loading={loading} onChange={setQuery} - onKeyDown={handleKeyDown} /> void - onKeyDown?: React.KeyboardEventHandler } const SearchInput = forwardRef( - function SearchInput({ query, loading, onChange, onKeyDown }, ref) { + function SearchInput({ query, loading, onChange }, ref) { return (
{createElement(presentation.TypeIcon, { @@ -402,7 +487,7 @@ function SearchResultRow({
- +
) } @@ -467,7 +552,7 @@ function SearchContent({ {hasResults && ( <> -
+
{results.map((result, i) => ( { }, id) } +async function installGlobalSearchResultsHarness(page: Page): Promise { + await page.waitForFunction(() => Boolean(window.__mockHandlers?.search_vault)) + await page.evaluate((results) => { + type Handler = (args?: Record) => unknown + const handlers = window.__mockHandlers as Record + Object.defineProperty(window, '__mockHandlers', { + configurable: true, + get: () => handlers, + set: (nextHandlers) => Object.assign(handlers, nextHandlers), + }) + handlers.search_vault = () => ({ results, elapsed_ms: 1 }) + }, GLOBAL_SEARCH_RESULTS) +} + +function searchResultRow(page: Page, title: string) { + return page.locator('[role="option"]').filter({ hasText: title }).first() +} + +async function expectSelectedSearchResult(page: Page, title: string): Promise { + await expect(searchResultRow(page, title)).toHaveClass(/bg-accent/) +} + test.describe('keyboard command routing', () => { test.beforeEach(() => { tempVaultDir = createFixtureVaultCopy() @@ -135,6 +164,35 @@ test.describe('keyboard command routing', () => { await expect(page.locator('input[placeholder="Search notes..."]')).toBeFocused() }) + test('global search arrow keys move one result at a time @smoke', async ({ page }) => { + await openFixtureVaultDesktopHarness(page, tempVaultDir) + await installGlobalSearchResultsHarness(page) + + await page.locator('body').click() + await dispatchShortcutEvent(page, { + key: 'f', + code: 'KeyF', + ctrlKey: false, + metaKey: true, + shiftKey: true, + altKey: false, + bubbles: true, + cancelable: true, + }) + const input = page.locator(GLOBAL_SEARCH_INPUT) + await expect(input).toBeVisible({ timeout: 5_000 }) + await input.fill('search') + await expect(searchResultRow(page, 'Third Search Result')).toBeVisible({ timeout: 5_000 }) + + await expectSelectedSearchResult(page, 'First Search Result') + await page.keyboard.press('ArrowDown') + await expectSelectedSearchResult(page, 'Second Search Result') + await page.keyboard.press('ArrowDown') + await expectSelectedSearchResult(page, 'Third Search Result') + await page.keyboard.press('ArrowUp') + await expectSelectedSearchResult(page, 'Second Search Result') + }) + test('desktop menu-command bridge toggles organized state through the shared command path @smoke', async ({ page }) => { await openAlphaProjectInEditor(page)