Skip to content

Commit 48c2c2d

Browse files
big-guyclaude
andcommitted
revert(review): single-file diff view, keep cross-file Find
The stacked all-files view mounted/disposed many Monaco diff editors as you scrolled (each createDiffEditor is heavy, plus automaticLayout polling and the virtualization mount/unmount hysteresis), making large reviews sluggish. Go back to one Monaco editor at a time — the file the reviewer selects — while keeping everything added since: multi-line comments, copy-path, open-in-editor, and the Cmd+F search across all files. ReviewDiffPane drops the IntersectionObserver virtualization, the auto-height host, per-file collapse, and the deleted/large click-to-reveal placeholders; the editor fills the pane and owns its own scroll again, and reveal uses revealLineInCenter. ReviewPane renders a single ReviewDiffPane for the selected file and restores the selectedFileObj/fileComments memos. Find is unchanged except that navigating to a match now switches the selected file first, then reveals the line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4a4668a commit 48c2c2d

2 files changed

Lines changed: 70 additions & 278 deletions

File tree

src/renderer/components/ReviewDiffPane.tsx

Lines changed: 22 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import { useState, useEffect, useCallback, useRef, type RefObject } from 'react'
1+
import { useState, useEffect, useCallback, useRef } from 'react'
22
import { createRoot, type Root } from 'react-dom/client'
33
import * as monaco from 'monaco-editor'
44
import ReactMarkdown from 'react-markdown'
55
import remarkGfm from 'remark-gfm'
66
import rehypeRaw from 'rehype-raw'
77
import rehypeSanitize from 'rehype-sanitize'
8-
import { Check, CheckCheck, ChevronDown, ChevronRight, Copy, FoldVertical, MessagesSquare, Pencil, Reply, UnfoldVertical } from 'lucide-react'
8+
import { Check, CheckCheck, Copy, FoldVertical, MessagesSquare, Pencil, Reply, UnfoldVertical } from 'lucide-react'
99
import type { FileDiffSides, ChangedFile } from '../types'
1010
import type { ReviewComment } from './ReviewFileTree'
1111
import { MonacoDiffEditor } from './MonacoDiffEditor'
@@ -31,16 +31,9 @@ interface ReviewDiffPaneProps {
3131
ignoreTrimWhitespace: boolean
3232
/** True when the review tab is active/visible — gates the `c` shortcut. */
3333
active?: boolean
34-
/** The scroll container that stacks all the file sections. Used as the
35-
* IntersectionObserver root so this section lazy-mounts its Monaco editor
36-
* only when it nears the viewport. */
37-
scrollRoot?: RefObject<HTMLElement | null>
3834
/** Scroll the diff to this line when it matches the current file. Used by
39-
* the comment list to jump to a comment. */
35+
* the comment list / find to jump to a line. */
4036
revealTarget?: { filePath: string; line: number; nonce: number } | null
41-
/** Collapsed sections render just the header (no diff editor). */
42-
collapsed?: boolean
43-
onToggleCollapsed?: () => void
4437
onToggleReviewed: () => void
4538
onAddComment: (lineNumber: number, body: string, startLine?: number) => void
4639
onDeleteComment: (id: string) => void
@@ -65,17 +58,6 @@ const STATUS_LABEL: Record<ChangedFile['status'], string> = {
6558
untracked: 'Untracked'
6659
}
6760

68-
// Diffs with more than this many changed lines are withheld behind a
69-
// click-to-reveal placeholder so the stacked view stays light by default.
70-
const LARGE_DIFF_LINES = 600
71-
72-
/** Why a file's diff is hidden by default (null = shown normally). */
73-
function withheldReason(file: ChangedFile): 'deleted' | 'large' | null {
74-
if (file.status === 'deleted') return 'deleted'
75-
if ((file.additions ?? 0) + (file.deletions ?? 0) > LARGE_DIFF_LINES) return 'large'
76-
return null
77-
}
78-
7961
const STATUS_COLOR: Record<ChangedFile['status'], string> = {
8062
added: 'text-success',
8163
modified: 'text-warning',
@@ -603,10 +585,7 @@ export function ReviewDiffPane({
603585
sideBySide,
604586
ignoreTrimWhitespace,
605587
active,
606-
scrollRoot,
607588
revealTarget,
608-
collapsed = false,
609-
onToggleCollapsed,
610589
onToggleReviewed,
611590
onAddComment,
612591
onDeleteComment,
@@ -618,18 +597,6 @@ export function ReviewDiffPane({
618597
}: ReviewDiffPaneProps): JSX.Element {
619598
const backend = useBackend()
620599
const settings = useSettings()
621-
// Lazy-mount: the Monaco editor is only constructed once this section has
622-
// scrolled near the viewport (stays mounted afterwards). `contentHeight`
623-
// is the editor's reported height in auto-height mode; before mount we show
624-
// a placeholder sized from the file's +/- counts so scroll stays stable.
625-
const rootRef = useRef<HTMLDivElement | null>(null)
626-
const isHoveredRef = useRef(false)
627-
const [hasBeenNear, setHasBeenNear] = useState(false)
628-
const [mounted, setMounted] = useState(false)
629-
const [contentHeight, setContentHeight] = useState(0)
630-
// Deleted files and very large diffs default to a click-to-reveal
631-
// placeholder rather than rendering the whole thing.
632-
const [revealed, setRevealed] = useState(false)
633600
const [sides, setSides] = useState<FileDiffSides | null>(null)
634601
const [loading, setLoading] = useState(false)
635602
// The line a pending comment input is anchored to (its end line). A separate
@@ -677,50 +644,6 @@ export function ReviewDiffPane({
677644
})
678645
}, [])
679646

680-
// Virtualize: mount the Monaco editor when this section scrolls within
681-
// ~800px of the viewport, unmount it once it's more than ~2000px away. The
682-
// hysteresis band (800–2000px) keeps sections near the edge from
683-
// thrash-cycling. `hasBeenNear` (latched) gates the one-time diff fetch so
684-
// the cached sides survive unmount/remount; `mounted` toggles the editor.
685-
useEffect(() => {
686-
const el = rootRef.current
687-
if (!el) return
688-
const root = scrollRoot?.current ?? null
689-
const mountIO = new IntersectionObserver(
690-
(entries) => {
691-
if (entries.some((e) => e.isIntersecting)) {
692-
setHasBeenNear(true)
693-
setMounted(true)
694-
}
695-
},
696-
{ root, rootMargin: '800px 0px' }
697-
)
698-
const unmountIO = new IntersectionObserver(
699-
(entries) => {
700-
if (entries.every((e) => !e.isIntersecting)) setMounted(false)
701-
},
702-
{ root, rootMargin: '2000px 0px' }
703-
)
704-
mountIO.observe(el)
705-
unmountIO.observe(el)
706-
return () => {
707-
mountIO.disconnect()
708-
unmountIO.disconnect()
709-
}
710-
}, [scrollRoot])
711-
712-
// When the editor is unmounted (scrolled far away), drop our handle and
713-
// tear down the comment view-zone React roots so they don't leak; the
714-
// measured height stays in `contentHeight` so the placeholder holds the
715-
// scroll position. Comments re-render on remount via the view-zone effect.
716-
useEffect(() => {
717-
if (mounted) return
718-
for (const z of viewZonesRef.current) queueMicrotask(() => z.root.unmount())
719-
viewZonesRef.current = []
720-
editorRef.current = null
721-
decorationsRef.current = null
722-
}, [mounted])
723-
724647
// Highlight the line span each multi-line comment (and the pending input
725648
// range) covers, so the reader sees what a range comment refers to.
726649
useEffect(() => {
@@ -748,12 +671,10 @@ export function ReviewDiffPane({
748671
}, [comments, commentLine, commentStartLine, editorNonce])
749672

750673
useEffect(() => {
751-
if (!file || !hasBeenNear) {
752-
if (!file) setSides(null)
674+
if (!file) {
675+
setSides(null)
753676
return
754677
}
755-
// Don't fetch withheld (deleted / very large) diffs until the user opts in.
756-
if (withheldReason(file) && !revealed) return
757678
let cancelled = false
758679
setLoading(true)
759680
setCommentLine(null)
@@ -786,7 +707,7 @@ export function ReviewDiffPane({
786707
return () => {
787708
cancelled = true
788709
}
789-
}, [hasBeenNear, revealed, worktreePath, file?.path, file?.staged, mode, commitHash, commitRange?.fromHash, commitRange?.toHash])
710+
}, [worktreePath, file?.path, file?.staged, mode, commitHash, commitRange?.fromHash, commitRange?.toHash])
790711

791712
const clearViewZones = useCallback(() => {
792713
const editor = editorRef.current
@@ -947,31 +868,19 @@ export function ReviewDiffPane({
947868
}
948869
}, [])
949870

950-
// Reveal a specific line when the comment list / find navigates here. In
951-
// the stacked auto-height view the editor owns no scroll, so we scroll the
952-
// OUTER container to the line's pixel offset (getTopForLineNumber accounts
953-
// for collapsed regions + view zones) and briefly flash the line. Keyed on
954-
// the request nonce + editor mount so it fires once this file's editor is
955-
// ready (it may have just mounted from the scroll).
871+
// Reveal a specific line when the comment list / find navigates here. The
872+
// editor owns its own scroll, so center the line and briefly flash it. Keyed
873+
// on the request nonce + editor mount so it fires once this file's editor is
874+
// ready (it may have just remounted from a file switch).
956875
useEffect(() => {
957876
if (!revealTarget || !file || revealTarget.filePath !== file.path) return
958877
const editor = editorRef.current
959-
const container = scrollRoot?.current
960878
if (!editor) return
961879
const line = Math.max(1, revealTarget.line)
962880
const modEd = editor.getModifiedEditor()
963881
let flash: monaco.editor.IEditorDecorationsCollection | null = null
964882
const doReveal = (): void => {
965-
if (container) {
966-
const node = modEd.getDomNode()
967-
if (node) {
968-
const cRect = container.getBoundingClientRect()
969-
const eRect = node.getBoundingClientRect()
970-
const editorTop = eRect.top - cRect.top + container.scrollTop
971-
const lineTop = modEd.getTopForLineNumber(line)
972-
container.scrollTo({ top: Math.max(0, editorTop + lineTop - 96), behavior: 'smooth' })
973-
}
974-
}
883+
modEd.revealLineInCenter(line)
975884
if (!flash) {
976885
flash = modEd.createDecorationsCollection([
977886
{
@@ -992,7 +901,7 @@ export function ReviewDiffPane({
992901
clearTimeout(clearT)
993902
flash?.clear()
994903
}
995-
}, [revealTarget?.nonce, revealTarget?.filePath, editorNonce, file?.path, scrollRoot])
904+
}, [revealTarget?.nonce, revealTarget?.filePath, editorNonce, file?.path])
996905

997906
const handleReferenceLine = useCallback((lineNumber: number) => {
998907
setCommentLine(lineNumber)
@@ -1040,16 +949,14 @@ export function ReviewDiffPane({
1040949
)
1041950

1042951
// `c` opens a comment input on the hovered diff line, or a file-level
1043-
// comment (line 0) when the mouse isn't over a line. Every stacked section
1044-
// installs this listener, so it only acts when the mouse is over THIS
1045-
// section — otherwise N sections would all open an input at once. Bail only
1046-
// on a real form field (the inline comment box and the file filter), never
1047-
// on Monaco's own input.
952+
// comment (line 0) when the mouse isn't over a line. Window listener so it
953+
// works whether focus is in the diff or the file tree. Bail only on a real
954+
// form field (the inline comment box and the file filter), never on Monaco's
955+
// own input.
1048956
useEffect(() => {
1049957
if (!file || !active) return
1050958
const onKey = (e: KeyboardEvent): void => {
1051959
if (e.key !== 'c' || e.metaKey || e.ctrlKey || e.altKey) return
1052-
if (!isHoveredRef.current) return
1053960
const el = e.target as HTMLElement | null
1054961
if (el instanceof HTMLInputElement) return
1055962
if (el instanceof HTMLTextAreaElement && !el.classList.contains('inputarea')) return
@@ -1078,42 +985,10 @@ export function ReviewDiffPane({
1078985
)
1079986
}
1080987

1081-
// Auto-height host: use the editor's reported content height once mounted,
1082-
// else a placeholder estimate from the file's +/- counts so the stacked
1083-
// scroll doesn't jump when this section mounts.
1084-
const lineH = Math.round(scaledEditorFontSize(settings.terminalFontSize, settings.uiScale) * 1.5)
1085-
const estimateHeight = Math.min(
1086-
1600,
1087-
Math.max(160, ((file.additions ?? 0) + (file.deletions ?? 0) + 8) * lineH)
1088-
)
1089-
const hostHeight = contentHeight > 0 ? contentHeight : estimateHeight
1090-
const reason = withheldReason(file)
1091-
const withheld = reason !== null && !revealed
1092-
1093988
return (
1094-
<div
1095-
ref={rootRef}
1096-
className="flex flex-col"
1097-
onMouseEnter={() => {
1098-
isHoveredRef.current = true
1099-
}}
1100-
onMouseLeave={() => {
1101-
isHoveredRef.current = false
1102-
}}
1103-
>
989+
<div className="flex flex-col h-full">
1104990
{/* File header */}
1105-
<div className="flex items-center gap-2 px-3 py-2 border-b border-border bg-panel sticky top-0 z-20">
1106-
{onToggleCollapsed && (
1107-
<button
1108-
onClick={onToggleCollapsed}
1109-
aria-label={collapsed ? 'Expand diff' : 'Collapse diff'}
1110-
aria-expanded={!collapsed}
1111-
className="shrink-0 text-faint hover:text-fg cursor-pointer"
1112-
>
1113-
{collapsed ? <ChevronRight className="icon-sm" /> : <ChevronDown className="icon-sm" />}
1114-
</button>
1115-
)}
1116-
991+
<div className="flex items-center gap-2 px-3 py-2 border-b border-border bg-panel shrink-0">
1117992
<Tooltip label={copiedPath ? 'Copied!' : 'Copy file path'}>
1118993
<button
1119994
onClick={() => {
@@ -1216,26 +1091,10 @@ export function ReviewDiffPane({
12161091
)}
12171092
</div>
12181093

1219-
{/* Diff with inline comments via view zones. Auto-height: the editor
1220-
grows to its content and the stacked container owns the scroll.
1221-
Collapsed sections render just the header; deleted/large diffs are
1222-
withheld behind a click-to-reveal row. */}
1223-
{!collapsed && withheld && (
1224-
<button
1225-
onClick={() => setRevealed(true)}
1226-
className="w-full text-left px-3 py-4 text-sm text-faint hover:text-fg hover:bg-panel/40 transition-colors cursor-pointer"
1227-
>
1228-
{reason === 'deleted'
1229-
? 'Diff not shown for deleted files by default'
1230-
: 'Large diffs are not shown by default'}
1231-
<span className="ml-2 text-dim">(click to show)</span>
1232-
</button>
1233-
)}
1234-
{!collapsed && !withheld && (
1235-
<div className="relative" style={{ height: hostHeight }}>
1236-
{!mounted ? (
1237-
<div className="absolute inset-0" aria-hidden />
1238-
) : loading ? (
1094+
{/* Diff with inline comments via view zones. The editor fills the pane
1095+
and owns its own vertical scroll. */}
1096+
<div className="flex-1 min-h-0 relative">
1097+
{loading ? (
12391098
<div className="absolute inset-0 flex items-center justify-center text-faint text-sm">
12401099
Loading diff...
12411100
</div>
@@ -1258,16 +1117,13 @@ export function ReviewDiffPane({
12581117
fontFamily={settings.terminalFontFamily || undefined}
12591118
fontSize={scaledEditorFontSize(settings.terminalFontSize, settings.uiScale)}
12601119
wordWrap={wordWrap}
1261-
autoHeight
1262-
onContentHeight={setContentHeight}
12631120
onReferenceLine={handleReferenceLine}
12641121
onEditorMount={handleEditorMount}
12651122
glyphClassName="comment-line-glyph"
12661123
glyphHoverMessage="Add a comment on this line"
12671124
/>
12681125
) : null}
12691126
</div>
1270-
)}
12711127
</div>
12721128
)
12731129
}

0 commit comments

Comments
 (0)