diff viewer: add file indicator, scroll-down navigation, keyboard hints

Philip Zeyliger created

Prompt: In this view, in desktop land, the file indicator should have a (1/10) or whatever to indicate that it's fine 1 of 10. Also, when we click next change, we should navigate from where the user is looking at now, down. It should never scroll up or to a previous file; always "down" or to the next file. When the user navigatesr to the diff view, we should already be scrolled to the first "diff" not the top of the file. When the user first opens the diff view, on desktop, we should have a toast that tells them about the keyboard navigation, that disappears after a second.

- Add file indicator (1/N) next to file selector showing current file position
- Change 'next change' to only navigate down/forward, never scroll up
- Auto-scroll to first diff when opening a file
- Show keyboard navigation hint toast on first open (desktop only)
- Add CSS for new file indicator and hint toast styling

Change summary

ui/src/components/DiffViewer.tsx | 89 ++++++++++++++++++++++++++-------
ui/src/styles.css                | 26 +++++++++
2 files changed, 95 insertions(+), 20 deletions(-)

Detailed changes

ui/src/components/DiffViewer.tsx 🔗

@@ -104,6 +104,8 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
   } | null>(null);
   const [commentText, setCommentText] = useState("");
   const [mode, setMode] = useState<ViewMode>("comment");
+  const [showKeyboardHint, setShowKeyboardHint] = useState(false);
+  const hasShownKeyboardHint = useRef(false);
 
   const [isMobile, setIsMobile] = useState(window.innerWidth < 768);
   const editorContainerRef = useRef<HTMLDivElement>(null);
@@ -156,6 +158,16 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
     }
   }, [isOpen, monacoLoaded]);
 
+  // Show keyboard hint toast on first open (desktop only)
+  useEffect(() => {
+    if (isOpen && !isMobile && !hasShownKeyboardHint.current && fileDiff) {
+      hasShownKeyboardHint.current = true;
+      setShowKeyboardHint(true);
+      const timer = setTimeout(() => setShowKeyboardHint(false), 6000);
+      return () => clearTimeout(timer);
+    }
+  }, [isOpen, isMobile, fileDiff]);
+
   // Load diffs when viewer opens, reset state when it closes
   useEffect(() => {
     if (isOpen && cwd) {
@@ -260,6 +272,20 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
 
     editorRef.current = diffEditor;
 
+    // Auto-scroll to first diff after editor is ready
+    // Use setTimeout to allow Monaco to compute the diff
+    setTimeout(() => {
+      const changes = diffEditor.getLineChanges();
+      if (changes && changes.length > 0) {
+        const firstChange = changes[0];
+        const targetLine = firstChange.modifiedStartLineNumber || 1;
+        const editor = diffEditor.getModifiedEditor();
+        editor.revealLineInCenter(targetLine);
+        editor.setPosition({ lineNumber: targetLine, column: 1 });
+        setCurrentChangeIndex(0);
+      }
+    }, 100);
+
     // Add click handler for commenting - clicking on a line in comment mode opens dialog
     const modifiedEditor = diffEditor.getModifiedEditor();
 
@@ -458,14 +484,26 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
     }
 
     const modifiedEditor = editorRef.current.getModifiedEditor();
-    const nextIdx = currentChangeIndex + 1;
+    const visibleRanges = modifiedEditor.getVisibleRanges();
+    const viewBottom = visibleRanges.length > 0 ? visibleRanges[0].endLineNumber : 0;
+
+    // Find the next change that starts below the current view
+    // This ensures we always move "down" and never scroll up
+    let nextIdx = -1;
+    for (let i = 0; i < changes.length; i++) {
+      const changeLine = changes[i].modifiedStartLineNumber || 1;
+      if (changeLine > viewBottom) {
+        nextIdx = i;
+        break;
+      }
+    }
 
-    if (nextIdx >= changes.length) {
-      // At end of file, try to go to next file
+    if (nextIdx === -1) {
+      // No more changes below current view, try to go to next file
       if (goToNextFile()) {
         return;
       }
-      // No next file, stay at last change
+      // No next file, stay where we are
       return;
     }
 
@@ -474,7 +512,7 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
     modifiedEditor.revealLineInCenter(targetLine);
     modifiedEditor.setPosition({ lineNumber: targetLine, column: 1 });
     setCurrentChangeIndex(nextIdx);
-  }, [currentChangeIndex, goToNextFile]);
+  }, [goToNextFile]);
 
   const goToPreviousChange = useCallback(() => {
     if (!editorRef.current) return;
@@ -707,22 +745,28 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
     </select>
   );
 
+  const fileIndexIndicator =
+    files.length > 1 && currentFileIndex >= 0 ? `(${currentFileIndex + 1}/${files.length})` : null;
+
   const fileSelector = (
-    <select
-      value={selectedFile || ""}
-      onChange={(e) => setSelectedFile(e.target.value || null)}
-      className="diff-viewer-select"
-      disabled={files.length === 0}
-    >
-      <option value="">{files.length === 0 ? "No files" : "Choose file..."}</option>
-      {files.map((file) => (
-        <option key={file.path} value={file.path}>
-          {getStatusSymbol(file.status)} {file.path}
-          {file.additions > 0 && ` (+${file.additions})`}
-          {file.deletions > 0 && ` (-${file.deletions})`}
-        </option>
-      ))}
-    </select>
+    <div className="diff-viewer-file-selector-wrapper">
+      <select
+        value={selectedFile || ""}
+        onChange={(e) => setSelectedFile(e.target.value || null)}
+        className="diff-viewer-select"
+        disabled={files.length === 0}
+      >
+        <option value="">{files.length === 0 ? "No files" : "Choose file..."}</option>
+        {files.map((file) => (
+          <option key={file.path} value={file.path}>
+            {getStatusSymbol(file.status)} {file.path}
+            {file.additions > 0 && ` (+${file.additions})`}
+            {file.deletions > 0 && ` (-${file.deletions})`}
+          </option>
+        ))}
+      </select>
+      {fileIndexIndicator && <span className="diff-viewer-file-index">{fileIndexIndicator}</span>}
+    </div>
   );
 
   const modeToggle = (
@@ -792,6 +836,11 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit }
             {saveStatus === "error" && "❌ Error saving"}
           </div>
         )}
+        {showKeyboardHint && (
+          <div className="diff-viewer-toast diff-viewer-toast-hint">
+            ⌨️ Use . , for next/prev change, &lt; &gt; for files
+          </div>
+        )}
 
         {/* Header - different layout for desktop vs mobile */}
         {isMobile ? (

ui/src/styles.css 🔗

@@ -2707,6 +2707,26 @@ svg {
   max-width: none;
 }
 
+.diff-viewer-file-selector-wrapper {
+  display: flex;
+  align-items: center;
+  gap: 0.5rem;
+  flex: 1;
+  min-width: 0;
+}
+
+.diff-viewer-file-selector-wrapper .diff-viewer-select {
+  flex: 1;
+  min-width: 0;
+}
+
+.diff-viewer-file-index {
+  font-size: 0.75rem;
+  color: var(--text-secondary);
+  white-space: nowrap;
+  flex-shrink: 0;
+}
+
 .diff-viewer-controls-row {
   display: flex;
   align-items: center;
@@ -2853,6 +2873,12 @@ svg {
   background: #d32f2f;
 }
 
+.diff-viewer-toast-hint {
+  background: var(--bg-tertiary);
+  color: var(--text-primary);
+  border: 1px solid var(--border-color);
+}
+
 @keyframes toast-fade-in {
   from {
     opacity: 0;