diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0e0971b74..22d60bc2f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -53,6 +53,7 @@ jobs: packages/ui/components/MarkdownEditor.extensions.test.tsx packages/ui/components/sidebar/FileBrowser.test.ts packages/editor/editableDocumentsHook.test.tsx + packages/review-editor/hooks/useReviewSearch.test.tsx packages/ui/components/AnnotationPanel.props.test.tsx packages/ui/components/Viewer.consumer.test.tsx packages/ui/components/InlineMarkdown.seam.test.tsx diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index e17c25b42..6641c21de 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -34,7 +34,12 @@ import { useAIChat } from './hooks/useAIChat'; import { toast, Toaster } from 'sonner'; import { useCodeNav, type CodeNavRequest } from './hooks/useCodeNav'; import { extractLinesFromPatch } from './utils/patchParser'; -import { isTypingTarget, useReviewSearch, type ReviewSearchMatch } from './hooks/useReviewSearch'; +import { + shouldHandleReviewSearchShortcut, + isTypingTarget, + useReviewSearch, + type ReviewSearchMatch, +} from './hooks/useReviewSearch'; import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations'; import { useExternalAnnotations } from '@plannotator/ui/hooks/useExternalAnnotations'; import { useAgentJobs, jobMatchesReviewContext } from '@plannotator/ui/hooks/useAgentJobs'; @@ -1193,7 +1198,13 @@ const ReviewApp: React.FC = () => { // Bail while the guide takeover is open (file tree isn't rendered) and // don't intercept in the Commits view (its rail has no search input) — // in both cases capturing the key would mutate hidden state or no-op. - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'f' && !isTypingTarget(e.target)) { + // Let the same shortcut reselect the current query when search already + // has focus, while preserving native shortcuts in every other input. + if ( + (e.metaKey || e.ctrlKey) + && e.key.toLowerCase() === 'f' + && shouldHandleReviewSearchShortcut(e.target, searchInputRef.current) + ) { if (guideOpen) return; if (hasSearchableFiles && !showCommitsPanel) { e.preventDefault(); diff --git a/packages/review-editor/hooks/useReviewSearch.test.tsx b/packages/review-editor/hooks/useReviewSearch.test.tsx new file mode 100644 index 000000000..58351044c --- /dev/null +++ b/packages/review-editor/hooks/useReviewSearch.test.tsx @@ -0,0 +1,102 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import React from 'react'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { + shouldHandleReviewSearchShortcut, + useReviewSearch, + type UseReviewSearchResult, +} from './useReviewSearch'; + +const hasDom = typeof document !== 'undefined'; + +let host: HTMLDivElement | null = null; +let root: Root | null = null; +let latest: UseReviewSearchResult | null = null; + +function Harness() { + const search = useReviewSearch({ + files: [], + activeFilePath: null, + }); + latest = search; + + return ( + search.handleSearchInputChange(event.target.value)} + /> + ); +} + +async function mountHarness(): Promise { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + + await act(async () => { + root!.render(); + }); + + const input = host.querySelector('input'); + if (!input) throw new Error('search input did not render'); + return input; +} + +async function openSearch(): Promise { + await act(async () => { + latest!.openSearch(); + await new Promise(resolve => requestAnimationFrame(() => resolve())); + }); +} + +function fakeElement(tagName: string, isContentEditable = false): HTMLElement { + return { tagName, isContentEditable } as HTMLElement; +} + +afterEach(async () => { + if (root) { + await act(async () => { + root!.unmount(); + }); + } + host?.remove(); + host = null; + root = null; + latest = null; +}); + +describe('useReviewSearch', () => { + test('handles shortcuts outside editable controls and inside the review search input', () => { + const searchInput = fakeElement('INPUT') as HTMLInputElement; + + expect(shouldHandleReviewSearchShortcut(searchInput, searchInput)).toBe(true); + expect(shouldHandleReviewSearchShortcut(fakeElement('INPUT'), searchInput)).toBe(false); + expect(shouldHandleReviewSearchShortcut(fakeElement('TEXTAREA'), searchInput)).toBe(false); + expect(shouldHandleReviewSearchShortcut(fakeElement('DIV', true), searchInput)).toBe(false); + expect(shouldHandleReviewSearchShortcut(fakeElement('DIV'), searchInput)).toBe(true); + }); + + test.skipIf(!hasDom)('selects the existing query whenever search is opened again', async () => { + const input = await mountHarness(); + + await act(async () => { + latest!.handleSearchInputChange('firstFunction'); + }); + + input.blur(); + input.setSelectionRange(input.value.length, input.value.length); + await openSearch(); + + expect(document.activeElement).toBe(input); + expect(input.selectionStart).toBe(0); + expect(input.selectionEnd).toBe(input.value.length); + + input.setSelectionRange(input.value.length, input.value.length); + await openSearch(); + + expect(input.selectionStart).toBe(0); + expect(input.selectionEnd).toBe(input.value.length); + }); +}); diff --git a/packages/review-editor/hooks/useReviewSearch.ts b/packages/review-editor/hooks/useReviewSearch.ts index 593343ead..79860865c 100644 --- a/packages/review-editor/hooks/useReviewSearch.ts +++ b/packages/review-editor/hooks/useReviewSearch.ts @@ -20,6 +20,13 @@ export function isTypingTarget(target: EventTarget | null): boolean { return tagName === 'INPUT' || tagName === 'TEXTAREA' || element.isContentEditable; } +export function shouldHandleReviewSearchShortcut( + target: EventTarget | null, + searchInput: HTMLInputElement | null, +): boolean { + return !isTypingTarget(target) || target === searchInput; +} + interface UseReviewSearchOptions { files: ReviewSearchableDiffFile[]; activeFilePath: string | null; @@ -72,7 +79,12 @@ export function useReviewSearch({ const openSearch = useCallback(() => { setIsSearchOpen(true); - requestAnimationFrame(() => searchInputRef.current?.focus()); + requestAnimationFrame(() => { + const input = searchInputRef.current; + if (!input) return; + input.focus(); + input.select(); + }); }, []); const closeSearch = useCallback(() => {