From 0ac3d868e5ede5aab84cde8358c0a18ccdfb5e8b Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Wed, 7 Jan 2026 12:39:45 -0800 Subject: [PATCH] diff viewer: add file indicator, scroll-down navigation, keyboard hints 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 --- ui/src/components/DiffViewer.tsx | 89 +++++++++++++++++++++++++------- ui/src/styles.css | 26 ++++++++++ 2 files changed, 95 insertions(+), 20 deletions(-) diff --git a/ui/src/components/DiffViewer.tsx b/ui/src/components/DiffViewer.tsx index bf432bd31a8b8330fb89c2bb44337096432a2796..b65d8462790e449bc9748bc28719c2282f22a1f1 100644 --- a/ui/src/components/DiffViewer.tsx +++ b/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("comment"); + const [showKeyboardHint, setShowKeyboardHint] = useState(false); + const hasShownKeyboardHint = useRef(false); const [isMobile, setIsMobile] = useState(window.innerWidth < 768); const editorContainerRef = useRef(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 } ); + const fileIndexIndicator = + files.length > 1 && currentFileIndex >= 0 ? `(${currentFileIndex + 1}/${files.length})` : null; + const fileSelector = ( - +
+ + {fileIndexIndicator && {fileIndexIndicator}} +
); const modeToggle = ( @@ -792,6 +836,11 @@ function DiffViewer({ cwd, isOpen, onClose, onCommentTextChange, initialCommit } {saveStatus === "error" && "❌ Error saving"} )} + {showKeyboardHint && ( +
+ ⌨️ Use . , for next/prev change, < > for files +
+ )} {/* Header - different layout for desktop vs mobile */} {isMobile ? ( diff --git a/ui/src/styles.css b/ui/src/styles.css index a7955e6b5eb42e191c53f67be37dcde33e62d515..a72047938348f7c9531df28c9a1423afb7ee8c14 100644 --- a/ui/src/styles.css +++ b/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;