diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index de8f7381..43e6bdf1 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -147,6 +147,8 @@ const ReviewApp: React.FC = () => { const { searchQuery, + debouncedSearchQuery, + isSearchPending, isSearchOpen, activeSearchMatchId, activeSearchMatch, @@ -336,7 +338,7 @@ const ReviewApp: React.FC = () => { } // Enter/F3 to step through search matches - if ((e.key === 'Enter' || e.key === 'F3') && searchMatches.length > 0 && !isTypingTarget(e.target)) { + if ((e.key === 'Enter' || e.key === 'F3') && searchMatches.length > 0 && !isSearchPending && !isTypingTarget(e.target)) { e.preventDefault(); stepSearchMatch(e.shiftKey ? -1 : 1); return; @@ -362,7 +364,7 @@ const ReviewApp: React.FC = () => { window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [showExportModal, showDestinationMenu, searchQuery, searchMatches, openSearch, stepSearchMatch, clearSearch, files, gitContext?.diffOptions]); + }, [showExportModal, showDestinationMenu, searchQuery, searchMatches, isSearchPending, openSearch, stepSearchMatch, clearSearch, files, gitContext?.diffOptions]); // Get annotations for active file const activeFileAnnotations = useMemo(() => { @@ -1337,6 +1339,7 @@ const ReviewApp: React.FC = () => { currentBranch={gitContext?.currentBranch} stagedFiles={stagedFiles} searchQuery={searchQuery} + isSearchPending={isSearchPending} searchInputRef={searchInputRef} onSearchChange={handleSearchInputChange} onSearchClear={clearSearch} @@ -1389,7 +1392,7 @@ const ReviewApp: React.FC = () => { onStage={() => stageFile(activeFile.path)} canStage={canStageFiles} stageError={stageError} - searchQuery={searchQuery} + searchQuery={isSearchPending ? '' : debouncedSearchQuery} searchMatches={activeFileSearchMatches} activeSearchMatchId={activeSearchMatchId} activeSearchMatch={activeSearchMatch?.filePath === activeFile.path ? activeSearchMatch : null} diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index 9b25456a..3d10e0f2 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -16,8 +16,10 @@ import { SuggestionModal } from './SuggestionModal'; import { type ReviewSearchMatch } from '../utils/reviewSearch'; import { applySearchHighlights, + clearSearchHighlights, getSearchRoots, retryScrollToSearchMatch, + swapActiveSearchHighlight, } from '../utils/reviewSearchHighlight'; interface DiffViewerProps { @@ -204,17 +206,33 @@ export const DiffViewer: React.FC = ({ return () => clearTimeout(timeoutId); }, [selectedAnnotationId]); - // Apply search highlights to diff lines (including inside shadow DOM) + // Apply search highlights to diff lines (including inside shadow DOM). + // The query is already debounced upstream (useReviewSearch), so this runs synchronously. + // activeSearchMatchId is NOT in deps — the swap effect handles that with O(1) updates. useEffect(() => { if (!containerRef.current) return; - const frameId = requestAnimationFrame(() => { - const roots = getSearchRoots(containerRef.current!); - roots.forEach(root => applySearchHighlights(root, searchQuery, searchMatches, activeSearchMatchId)); - }); + const query = searchQuery; + const matches = searchMatches; - return () => cancelAnimationFrame(frameId); - }, [searchQuery, searchMatches, activeSearchMatchId, filePath, diffStyle, augmentedDiff]); + if (!query.trim() || matches.length === 0) { + const roots = getSearchRoots(containerRef.current); + roots.forEach(root => clearSearchHighlights(root)); + return; + } + + const roots = getSearchRoots(containerRef.current); + roots.forEach(root => + applySearchHighlights(root, query, matches, activeSearchMatchId) + ); + }, [searchQuery, searchMatches, filePath, diffStyle, augmentedDiff]); + + // Swap active search highlight instantly when stepping between matches. + // This avoids a full rebuild just to change two elements' background color. + useEffect(() => { + if (!containerRef.current) return; + swapActiveSearchHighlight(containerRef.current, activeSearchMatchId); + }, [activeSearchMatchId]); // Scroll to active search match (with retry for lazy-rendered content) useEffect(() => { diff --git a/packages/review-editor/components/FileTree.tsx b/packages/review-editor/components/FileTree.tsx index 7dc23ca7..277ff467 100644 --- a/packages/review-editor/components/FileTree.tsx +++ b/packages/review-editor/components/FileTree.tsx @@ -27,6 +27,7 @@ interface FileTreeProps { currentBranch?: string; stagedFiles?: Set; searchQuery?: string; + isSearchPending?: boolean; searchInputRef?: React.RefObject; onSearchChange?: (value: string) => void; onSearchClear?: () => void; @@ -58,6 +59,7 @@ export const FileTree: React.FC = ({ currentBranch, stagedFiles, searchQuery = '', + isSearchPending, searchInputRef, onSearchChange, onSearchClear, @@ -166,7 +168,7 @@ export const FileTree: React.FC = ({ e.preventDefault(); return; } - if (e.key === 'Enter' && searchMatches.length > 0) { + if (e.key === 'Enter' && searchMatches.length > 0 && !isSearchPending) { e.preventDefault(); onStepSearchMatch?.(e.shiftKey ? -1 : 1); } @@ -184,7 +186,7 @@ export const FileTree: React.FC = ({ /> {searchQuery && (
- {searchQuery.trim() && ( + {searchQuery.trim() && !isSearchPending && ( {searchMatches.length} @@ -332,7 +334,11 @@ export const FileTree: React.FC = ({ {/* File tree or search results */}
{searchQuery.trim() ? ( - searchGroups.length > 0 ? ( + isSearchPending ? ( +
+ Searching… +
+ ) : searchGroups.length > 0 ? ( searchGroups.map((group) => ( = ({ // --- Search result components --- function highlightQuery(text: string, query: string) { - if (!query.trim()) return text; - const regex = new RegExp(`(${query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi'); + const trimmed = query.trim(); + if (!trimmed) return text; + const regex = new RegExp(`(${trimmed.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi'); const parts = text.split(regex); // split with a capturing group puts matches at odd indices (1, 3, 5...) return parts.map((part, i) => diff --git a/packages/review-editor/hooks/useReviewSearch.ts b/packages/review-editor/hooks/useReviewSearch.ts index 56f881b8..96921e59 100644 --- a/packages/review-editor/hooks/useReviewSearch.ts +++ b/packages/review-editor/hooks/useReviewSearch.ts @@ -1,7 +1,8 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { - findReviewSearchMatches, groupReviewSearchMatches, + buildSearchIndex, + findMatchesInIndex, type ReviewSearchMatch, type ReviewSearchableDiffFile, } from '../utils/reviewSearch'; @@ -33,14 +34,29 @@ export function useReviewSearch({ clearPendingSelection, }: UseReviewSearchOptions) { const [searchQuery, setSearchQuery] = useState(''); + const [debouncedSearchQuery, setDebouncedSearchQuery] = useState(''); const [isSearchOpen, setIsSearchOpen] = useState(false); const [activeSearchMatchId, setActiveSearchMatchId] = useState(null as string | null); const searchInputRef = useRef(null as HTMLInputElement | null); + useEffect(() => { + if (!searchQuery.trim()) { + setDebouncedSearchQuery(''); + return; + } + const timeoutId = setTimeout(() => setDebouncedSearchQuery(searchQuery), 200); + return () => clearTimeout(timeoutId); + }, [searchQuery]); + + const isSearchPending = searchQuery.trim() !== '' + && searchQuery.trim().toLowerCase() !== debouncedSearchQuery.trim().toLowerCase(); + + const searchIndex = useMemo(() => buildSearchIndex(files), [files]); + const searchMatches = useMemo(() => { - return findReviewSearchMatches(files, searchQuery); - }, [files, searchQuery]); + return findMatchesInIndex(searchIndex, debouncedSearchQuery); + }, [searchIndex, debouncedSearchQuery]); const searchGroups = useMemo(() => { return groupReviewSearchMatches(files, searchMatches); @@ -68,6 +84,7 @@ export function useReviewSearch({ const clearSearch = useCallback(() => { setSearchQuery(''); + setDebouncedSearchQuery(''); setActiveSearchMatchId(null); setIsSearchOpen(false); }, []); @@ -95,7 +112,7 @@ export function useReviewSearch({ }, [searchMatches, activeSearchMatchId]); useEffect(() => { - if (!searchQuery.trim() || searchMatches.length === 0) { + if (!debouncedSearchQuery.trim() || searchMatches.length === 0) { setActiveSearchMatchId(null); return; } @@ -106,7 +123,7 @@ export function useReviewSearch({ if (!stillExists) { setActiveSearchMatchId(searchMatches[0].id); } - }, [searchQuery, searchMatches, activeSearchMatchId]); + }, [debouncedSearchQuery, searchMatches, activeSearchMatchId]); useEffect(() => { if (!activeSearchMatch) return; @@ -120,6 +137,8 @@ export function useReviewSearch({ return { searchQuery, + debouncedSearchQuery, + isSearchPending, isSearchOpen, activeSearchMatchId, activeSearchMatch, diff --git a/packages/review-editor/utils/reviewSearch.ts b/packages/review-editor/utils/reviewSearch.ts index 9e1fffac..f10fe04d 100644 --- a/packages/review-editor/utils/reviewSearch.ts +++ b/packages/review-editor/utils/reviewSearch.ts @@ -42,12 +42,13 @@ export interface ReviewSearchFileGroup { matches: ReviewSearchMatch[]; } -interface SearchableLine { +export interface SearchableLine { filePath: string; side: ReviewSearchSide; lineNumber: number; altLineNumber?: number; text: string; + normalizedText: string; } function isDiffMetadataLine(line: string): boolean { @@ -84,6 +85,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl lineNumber: newLine, altLineNumber: oldLine, text, + normalizedText: text.toLowerCase(), }); continue; } @@ -95,6 +97,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl side: 'deletion', lineNumber: oldLine, text, + normalizedText: text.toLowerCase(), }); continue; } @@ -106,6 +109,7 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl side: 'addition', lineNumber: newLine, text, + normalizedText: text.toLowerCase(), }); } } @@ -113,6 +117,10 @@ function buildSearchableLinesForPatch(file: ReviewSearchableDiffFile): Searchabl return searchableLines; } +export function buildSearchIndex(files: ReviewSearchableDiffFile[]): SearchableLine[] { + return files.flatMap(file => buildSearchableLinesForPatch(file)); +} + function buildSnippet(text: string, start: number, end: number): string { const snippetStart = Math.max(0, start - SNIPPET_CONTEXT); const snippetEnd = Math.min(text.length, end + SNIPPET_CONTEXT); @@ -121,47 +129,46 @@ function buildSnippet(text: string, start: number, end: number): string { return `${prefix}${text.slice(snippetStart, snippetEnd)}${suffix}`; } -export function findReviewSearchMatches(files: ReviewSearchableDiffFile[], query: string): ReviewSearchMatch[] { +export function findMatchesInIndex(index: SearchableLine[], query: string): ReviewSearchMatch[] { const trimmedQuery = query.trim(); if (!trimmedQuery) return []; const normalizedQuery = trimmedQuery.toLowerCase(); const matches: ReviewSearchMatch[] = []; - files.forEach((file) => { - const searchableLines = buildSearchableLinesForPatch(file); + index.forEach((line) => { + let fromIndex = 0; + let matchCountForLine = 0; - searchableLines.forEach((line) => { - const normalizedText = line.text.toLowerCase(); - let fromIndex = 0; - let matchCountForLine = 0; + while (fromIndex <= line.normalizedText.length) { + const matchStart = line.normalizedText.indexOf(normalizedQuery, fromIndex); + if (matchStart === -1) break; - while (fromIndex <= normalizedText.length) { - const matchStart = normalizedText.indexOf(normalizedQuery, fromIndex); - if (matchStart === -1) break; + const matchEnd = matchStart + normalizedQuery.length; + matches.push({ + id: `${line.filePath}:${line.side}:${line.lineNumber}:${matchStart}:${matchCountForLine}`, + filePath: line.filePath, + side: line.side, + lineNumber: line.lineNumber, + altLineNumber: line.altLineNumber, + text: line.text, + matchStart, + matchEnd, + snippet: buildSnippet(line.text, matchStart, matchEnd), + }); - const matchEnd = matchStart + normalizedQuery.length; - matches.push({ - id: `${line.filePath}:${line.side}:${line.lineNumber}:${matchStart}:${matchCountForLine}`, - filePath: line.filePath, - side: line.side, - lineNumber: line.lineNumber, - altLineNumber: line.altLineNumber, - text: line.text, - matchStart, - matchEnd, - snippet: buildSnippet(line.text, matchStart, matchEnd), - }); - - matchCountForLine += 1; - fromIndex = matchStart + Math.max(1, normalizedQuery.length); - } - }); + matchCountForLine += 1; + fromIndex = matchStart + Math.max(1, normalizedQuery.length); + } }); return matches; } +export function findReviewSearchMatches(files: ReviewSearchableDiffFile[], query: string): ReviewSearchMatch[] { + return findMatchesInIndex(buildSearchIndex(files), query); +} + export function groupReviewSearchMatches( files: ReviewSearchableDiffFile[], matches: ReviewSearchMatch[], diff --git a/packages/review-editor/utils/reviewSearchHighlight.ts b/packages/review-editor/utils/reviewSearchHighlight.ts index 8f25aba8..48db10e8 100644 --- a/packages/review-editor/utils/reviewSearchHighlight.ts +++ b/packages/review-editor/utils/reviewSearchHighlight.ts @@ -28,7 +28,7 @@ export function getSearchRoots(root: ParentNode): ParentNode[] { return roots; } -function clearSearchHighlights(root: ParentNode) { +export function clearSearchHighlights(root: ParentNode) { const marks = root.querySelectorAll('mark[data-review-search-match]'); marks.forEach((mark) => { const parent = mark.parentNode; @@ -66,6 +66,11 @@ function decorateSearchMatch(mark: HTMLElement, isActive: boolean) { mark.style.borderRadius = '3px'; mark.style.padding = '0 1px'; mark.style.boxShadow = isActive ? ACTIVE_MATCH_RING : PASSIVE_MATCH_RING; + if (isActive) { + mark.dataset.reviewSearchActive = ''; + } else { + delete mark.dataset.reviewSearchActive; + } } function lineKey(match: ReviewSearchMatch): string { @@ -150,6 +155,25 @@ export function applySearchHighlights( }); } +export function swapActiveSearchHighlight( + container: HTMLElement, + newActiveId: string | null, +): void { + const roots = getSearchRoots(container); + for (const root of roots) { + const prev = root.querySelector('mark[data-review-search-active]') as HTMLElement | null; + if (prev) { + decorateSearchMatch(prev, false); + } + if (newActiveId) { + const next = root.querySelector(`mark[data-review-search-match="${newActiveId}"]`) as HTMLElement | null; + if (next) { + decorateSearchMatch(next, true); + } + } + } +} + export function scrollToSearchMatch(root: ParentNode, match: ReviewSearchMatch): boolean { const lineEl = root.querySelector(getLineSelector(match)) as HTMLElement | null; if (!lineEl) return false;