(bug-fix: 1984) Update search highlighter to use overlay (#2035)

ui/desktop/src/components/ChatView.tsx

@@ -388,7 +388,7 @@ export default function ChatView({
           />
         ) : (
           <ScrollArea ref={scrollRef} className="flex-1" autoScroll>
-            <SearchView scrollAreaRef={scrollRef}>
+            <SearchView>
               {filteredMessages.map((message, index) => (
                 <div key={message.id || index} className="mt-4 px-4">
                   {isUserMessage(message) ? (

ui/desktop/src/components/conversation/SearchBar.tsx

@@ -51,18 +51,17 @@ export const SearchBar: React.FC<SearchBarProps> = ({
 
   const handleKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
     if (event.key === 'ArrowUp') {
-      event.preventDefault();
-      onNavigate?.('prev');
+      handleNavigate('prev', event);
     } else if (event.key === 'ArrowDown') {
-      event.preventDefault();
-      onNavigate?.('next');
+      handleNavigate('next', event);
     } else if (event.key === 'Escape') {
       event.preventDefault();
       handleClose();
     }
   };
 
-  const handleNavigate = (direction: 'next' | 'prev') => {
+  const handleNavigate = (direction: 'next' | 'prev', e?: React.MouseEvent | KeyboardEvent) => {
+    e?.preventDefault();
     onNavigate?.(direction);
     inputRef.current?.focus();
   };
@@ -123,7 +122,7 @@ export const SearchBar: React.FC<SearchBarProps> = ({
           </button>
 
           <button
-            onClick={() => handleNavigate('prev')}
+            onClick={(e) => handleNavigate('prev', e)}
             className={`p-1 text-textSubtleInverse ${!searchResults || searchResults.count === 0 ? '' : 'hover:text-textStandardInverse'}`}
             title="Previous (↑)"
             disabled={!searchResults || searchResults.count === 0}
@@ -131,7 +130,7 @@ export const SearchBar: React.FC<SearchBarProps> = ({
             <ArrowUp className="h-5 w-5" />
           </button>
           <button
-            onClick={() => handleNavigate('next')}
+            onClick={(e) => handleNavigate('next', e)}
             className={`p-1 text-textSubtleInverse ${!searchResults || searchResults.count === 0 ? '' : 'hover:text-textStandardInverse'}`}
             title="Next (↓)"
             disabled={!searchResults || searchResults.count === 0}

ui/desktop/src/components/conversation/SearchView.tsx

@@ -1,7 +1,6 @@
 import React, { useState, useEffect, PropsWithChildren } from 'react';
 import { SearchBar } from './SearchBar';
 import { SearchHighlighter } from '../../utils/searchHighlighter';
-import { ScrollAreaHandle } from '../ui/scroll-area';
 import '../../styles/search.css';
 
 /**
@@ -10,8 +9,6 @@ import '../../styles/search.css';
 interface SearchViewProps {
   /** Optional CSS class name */
   className?: string;
-  /** Reference to the scroll area for navigation */
-  scrollAreaRef?: React.RefObject<ScrollAreaHandle>;
 }
 
 /**
@@ -21,7 +18,6 @@ interface SearchViewProps {
 export const SearchView: React.FC<PropsWithChildren<SearchViewProps>> = ({
   className = '',
   children,
-  scrollAreaRef,
 }) => {
   const [isSearchVisible, setIsSearchVisible] = useState(false);
   const [searchResults, setSearchResults] = useState<{
@@ -32,26 +28,41 @@ export const SearchView: React.FC<PropsWithChildren<SearchViewProps>> = ({
   const highlighterRef = React.useRef<SearchHighlighter | null>(null);
   const containerRef = React.useRef<HTMLDivElement | null>(null);
 
-  // temporarily disabling search for launch until issue https://github.com/block/goose/issues/1984 is resolved
-  //
-  // useEffect(() => {
-  //   const handleKeyDown = (e: KeyboardEvent) => {
-  //     if ((e.metaKey || e.ctrlKey) && e.key === 'f') {
-  //       e.preventDefault();
-  //       setIsSearchVisible(true);
-  //     }
-  //   };
-  //
-  //   window.addEventListener('keydown', handleKeyDown);
-  //   return () => {
-  //     window.removeEventListener('keydown', handleKeyDown);
-  //   };
-  // }, []);
+  // Clean up highlighter on unmount
+  useEffect(() => {
+    return () => {
+      if (highlighterRef.current) {
+        highlighterRef.current.destroy();
+        highlighterRef.current = null;
+      }
+    };
+  }, []);
 
+  useEffect(() => {
+    const handleKeyDown = (e: KeyboardEvent) => {
+      if ((e.metaKey || e.ctrlKey) && e.key === 'f') {
+        e.preventDefault();
+        setIsSearchVisible(true);
+      }
+    };
+
+    window.addEventListener('keydown', handleKeyDown);
+    return () => {
+      window.removeEventListener('keydown', handleKeyDown);
+    };
+  }, []);
+
+  /**
+   * Handles the search operation when a user enters a search term.
+   * @param term - The text to search for
+   * @param caseSensitive - Whether to perform a case-sensitive search
+   */
   const handleSearch = (term: string, caseSensitive: boolean) => {
     if (!term) {
       setSearchResults(null);
-      clearHighlights();
+      if (highlighterRef.current) {
+        highlighterRef.current.clearHighlights();
+      }
       return;
     }
 
@@ -59,36 +70,52 @@ export const SearchView: React.FC<PropsWithChildren<SearchViewProps>> = ({
     if (!container) return;
 
     if (!highlighterRef.current) {
-      highlighterRef.current = new SearchHighlighter(container);
+      highlighterRef.current = new SearchHighlighter(container, (count) => {
+        if (count > 0) {
+          setSearchResults((prev) => ({
+            currentIndex: prev?.currentIndex || 1,
+            count,
+          }));
+        } else {
+          setSearchResults(null);
+        }
+      });
     }
 
-    highlighterRef.current.clearHighlights();
-    highlighterRef.current.highlight(term, caseSensitive);
-
-    const marks = container.querySelectorAll('mark');
-    const count = marks.length;
+    const highlights = highlighterRef.current.highlight(term, caseSensitive);
+    const count = highlights.length;
 
     if (count > 0) {
       setSearchResults({
         currentIndex: 1,
-        count: count,
+        count,
       });
-      scrollToMatch(0);
+      highlighterRef.current.setCurrentMatch(0, true); // Explicitly scroll when setting initial match
     } else {
       setSearchResults(null);
     }
   };
 
+  /**
+   * Navigates between search results in the specified direction.
+   * @param direction - Direction to navigate ('next' or 'prev')
+   */
   const navigateResults = (direction: 'next' | 'prev') => {
-    if (!searchResults || searchResults.count === 0) return;
+    if (!searchResults || searchResults.count === 0 || !highlighterRef.current) return;
 
     let newIndex: number;
     const currentIdx = searchResults.currentIndex - 1; // Convert to 0-based
 
     if (direction === 'next') {
-      newIndex = (currentIdx + 1) % searchResults.count;
+      newIndex = currentIdx + 1;
+      if (newIndex >= searchResults.count) {
+        newIndex = 0;
+      }
     } else {
-      newIndex = (currentIdx - 1 + searchResults.count) % searchResults.count;
+      newIndex = currentIdx - 1;
+      if (newIndex < 0) {
+        newIndex = searchResults.count - 1;
+      }
     }
 
     setSearchResults({
@@ -96,59 +123,18 @@ export const SearchView: React.FC<PropsWithChildren<SearchViewProps>> = ({
       currentIndex: newIndex + 1,
     });
 
-    scrollToMatch(newIndex);
-  };
-
-  const scrollToMatch = (index: number) => {
-    if (!containerRef.current || !scrollAreaRef?.current) return;
-
-    const marks = containerRef.current.querySelectorAll('mark');
-    const mark = marks[index] as HTMLElement;
-    if (!mark) return;
-
-    // Update highlight
-    marks.forEach((m) => m.classList.remove('current'));
-    mark.classList.add('current');
-
-    // Find the viewport element
-    const viewport = mark.closest('[data-radix-scroll-area-viewport]') as HTMLElement;
-    if (!viewport) return;
-
-    // Get measurements
-    const viewportRect = viewport.getBoundingClientRect();
-    const markRect = mark.getBoundingClientRect();
-    const currentScrollTop = viewport.scrollTop;
-
-    // Calculate how far the element is from the top of the viewport
-    const elementRelativeToViewport = markRect.top - viewportRect.top;
-
-    // Calculate the new scroll position that would center the element
-    const targetPosition =
-      currentScrollTop + elementRelativeToViewport - (viewportRect.height - markRect.height) / 2;
-
-    // Ensure we don't scroll past the bottom
-    const maxScroll = viewport.scrollHeight - viewport.clientHeight;
-    const finalPosition = Math.max(0, Math.min(targetPosition, maxScroll));
-
-    // Use requestAnimationFrame to ensure DOM measurements are accurate
-    requestAnimationFrame(() => {
-      scrollAreaRef.current?.scrollToPosition({
-        top: finalPosition,
-        behavior: 'smooth',
-      });
-    });
-  };
-
-  const clearHighlights = () => {
-    if (highlighterRef.current) {
-      highlighterRef.current.clearHighlights();
-    }
+    highlighterRef.current.setCurrentMatch(newIndex, true); // Explicitly scroll when navigating
   };
 
+  /**
+   * Closes the search interface and clears all highlights.
+   */
   const handleCloseSearch = () => {
     setIsSearchVisible(false);
     setSearchResults(null);
-    clearHighlights();
+    if (highlighterRef.current) {
+      highlighterRef.current.clearHighlights();
+    }
   };
 
   return (