Skip to content

Commit dc9f0cb

Browse files
benvinegarbenvinegar
committed
Fix wrap toggle redraw and viewport anchoring
1 parent d17537b commit dc9f0cb

File tree

6 files changed

+489
-19
lines changed

6 files changed

+489
-19
lines changed

‎src/ui/App.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ function AppShell({
107107
const terminal = useTerminalDimensions();
108108
const filesScrollRef = useRef<ScrollBoxRenderable | null>(null);
109109
const diffScrollRef = useRef<ScrollBoxRenderable | null>(null);
110+
const wrapToggleScrollTopRef = useRef<number | null>(null);
110111
const [layoutMode, setLayoutMode] = useState<LayoutMode>(bootstrap.initialMode);
111112
const [themeId, setThemeId] = useState(
112113
() => resolveTheme(bootstrap.initialTheme, renderer.themeMode).id,
@@ -232,9 +233,10 @@ function AppShell({
232233
}, [maxFilesPaneWidth, showFilesPane]);
233234

234235
useEffect(() => {
235-
// Force an intermediate redraw when the shell geometry changes so pane relayout feels immediate.
236+
// Force an intermediate redraw when shell geometry or row-wrapping changes so pane relayout
237+
// feels immediate after toggling split/stack or line wrapping.
236238
renderer.intermediateRender();
237-
}, [renderer, resolvedLayout, showFilesPane, terminal.height, terminal.width]);
239+
}, [renderer, resolvedLayout, showFilesPane, terminal.height, terminal.width, wrapLines]);
238240

239241
useEffect(() => {
240242
if (!selectedFile && filteredFiles[0]) {
@@ -335,6 +337,7 @@ function AppShell({
335337

336338
/** Toggle whether diff code rows wrap instead of truncating to one terminal row. */
337339
const toggleLineWrap = () => {
340+
wrapToggleScrollTopRef.current = diffScrollRef.current?.scrollTop ?? 0;
338341
setWrapLines((current) => !current);
339342
};
340343

@@ -662,6 +665,11 @@ function AppShell({
662665
return;
663666
}
664667

668+
if (key.name === "w" || key.sequence === "w") {
669+
toggleLineWrap();
670+
return;
671+
}
672+
665673
return;
666674
}
667675

@@ -949,6 +957,7 @@ function AppShell({
949957
showLineNumbers={showLineNumbers}
950958
showHunkHeaders={showHunkHeaders}
951959
wrapLines={wrapLines}
960+
wrapToggleScrollTop={wrapToggleScrollTopRef.current}
952961
theme={activeTheme}
953962
width={diffPaneWidth}
954963
onOpenAgentNotesAtHunk={openAgentNotesAtHunk}

‎src/ui/components/panes/DiffPane.tsx‎

Lines changed: 150 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from "react";
1111
import type { DiffFile, LayoutMode } from "../../../core/types";
1212
import type { VisibleAgentNote } from "../../lib/agentAnnotations";
13-
import { measureDiffSectionMetrics } from "../../lib/sectionHeights";
13+
import { measureDiffSectionMetrics, type DiffSectionMetrics } from "../../lib/sectionHeights";
1414
import { diffHunkId, diffSectionId } from "../../lib/ids";
1515
import type { AppTheme } from "../../themes";
1616
import { DiffSection } from "./DiffSection";
@@ -19,6 +19,80 @@ import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/Ve
1919

2020
const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = [];
2121

22+
interface ViewportRowAnchor {
23+
fileId: string;
24+
rowKey: string;
25+
rowOffsetWithin: number;
26+
}
27+
28+
function findViewportRowAnchor(
29+
files: DiffFile[],
30+
sectionMetrics: DiffSectionMetrics[],
31+
scrollTop: number,
32+
) {
33+
let offsetY = 0;
34+
35+
for (let index = 0; index < files.length; index += 1) {
36+
if (index > 0) {
37+
offsetY += 1;
38+
}
39+
40+
offsetY += 1;
41+
const bodyTop = offsetY;
42+
const metrics = sectionMetrics[index];
43+
const bodyHeight = metrics?.bodyHeight ?? 0;
44+
const relativeTop = scrollTop - bodyTop;
45+
46+
if (relativeTop >= 0 && relativeTop < bodyHeight && metrics) {
47+
const rowMetric = metrics.rowMetrics.find(
48+
(candidate) =>
49+
relativeTop >= candidate.offset && relativeTop < candidate.offset + candidate.height,
50+
);
51+
if (rowMetric) {
52+
return {
53+
fileId: files[index]!.id,
54+
rowKey: rowMetric.key,
55+
rowOffsetWithin: relativeTop - rowMetric.offset,
56+
} satisfies ViewportRowAnchor;
57+
}
58+
}
59+
60+
offsetY = bodyTop + bodyHeight;
61+
}
62+
63+
return null;
64+
}
65+
66+
function resolveViewportRowAnchorTop(
67+
files: DiffFile[],
68+
sectionMetrics: DiffSectionMetrics[],
69+
anchor: ViewportRowAnchor,
70+
) {
71+
let offsetY = 0;
72+
73+
for (let index = 0; index < files.length; index += 1) {
74+
if (index > 0) {
75+
offsetY += 1;
76+
}
77+
78+
offsetY += 1;
79+
const bodyTop = offsetY;
80+
const file = files[index];
81+
const metrics = sectionMetrics[index];
82+
if (file?.id === anchor.fileId && metrics) {
83+
const rowMetric = metrics.rowMetricsByKey.get(anchor.rowKey);
84+
if (rowMetric) {
85+
return bodyTop + rowMetric.offset + Math.min(anchor.rowOffsetWithin, rowMetric.height - 1);
86+
}
87+
return bodyTop;
88+
}
89+
90+
offsetY = bodyTop + (metrics?.bodyHeight ?? 0);
91+
}
92+
93+
return 0;
94+
}
95+
2296
/** Render the main multi-file review stream. */
2397
export function DiffPane({
2498
diffContentWidth,
@@ -35,6 +109,7 @@ export function DiffPane({
35109
showLineNumbers,
36110
showHunkHeaders,
37111
wrapLines,
112+
wrapToggleScrollTop,
38113
theme,
39114
width,
40115
onOpenAgentNotesAtHunk,
@@ -54,6 +129,7 @@ export function DiffPane({
54129
showLineNumbers: boolean;
55130
showHunkHeaders: boolean;
56131
wrapLines: boolean;
132+
wrapToggleScrollTop: number | null;
57133
theme: AppTheme;
58134
width: number;
59135
onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void;
@@ -131,6 +207,10 @@ export function DiffPane({
131207
const [scrollViewport, setScrollViewport] = useState({ top: 0, height: 0 });
132208
const scrollbarRef = useRef<VerticalScrollbarHandle>(null);
133209
const prevScrollTopRef = useRef(0);
210+
const previousSectionMetricsRef = useRef<DiffSectionMetrics[] | null>(null);
211+
const previousFilesRef = useRef<DiffFile[]>(files);
212+
const previousWrapLinesRef = useRef(wrapLines);
213+
const suppressNextSelectionAutoScrollRef = useRef(false);
134214

135215
useEffect(() => {
136216
const updateViewport = () => {
@@ -156,8 +236,20 @@ export function DiffPane({
156236
}, [scrollRef]);
157237

158238
const baseSectionMetrics = useMemo(
159-
() => files.map((file) => measureDiffSectionMetrics(file, layout, showHunkHeaders, theme)),
160-
[files, layout, showHunkHeaders, theme],
239+
() =>
240+
files.map((file) =>
241+
measureDiffSectionMetrics(
242+
file,
243+
layout,
244+
showHunkHeaders,
245+
theme,
246+
EMPTY_VISIBLE_AGENT_NOTES,
247+
diffContentWidth,
248+
showLineNumbers,
249+
wrapLines,
250+
),
251+
),
252+
[diffContentWidth, files, layout, showHunkHeaders, showLineNumbers, theme, wrapLines],
161253
);
162254
const baseEstimatedBodyHeights = useMemo(
163255
() => baseSectionMetrics.map((metrics) => metrics.bodyHeight),
@@ -218,6 +310,8 @@ export function DiffPane({
218310
theme,
219311
visibleNotes,
220312
diffContentWidth,
313+
showLineNumbers,
314+
wrapLines,
221315
);
222316
}),
223317
[
@@ -226,8 +320,10 @@ export function DiffPane({
226320
files,
227321
layout,
228322
showHunkHeaders,
323+
showLineNumbers,
229324
theme,
230325
visibleAgentNotesByFile,
326+
wrapLines,
231327
],
232328
);
233329
const estimatedBodyHeights = useMemo(
@@ -306,6 +402,53 @@ export function DiffPane({
306402
}, [estimatedBodyHeights, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]);
307403

308404
useLayoutEffect(() => {
405+
const wrapChanged = previousWrapLinesRef.current !== wrapLines;
406+
const previousSectionMetrics = previousSectionMetricsRef.current;
407+
const previousFiles = previousFilesRef.current;
408+
409+
if (wrapChanged && previousSectionMetrics && previousFiles.length > 0) {
410+
const previousScrollTop = Math.max(
411+
wrapToggleScrollTop ?? 0,
412+
prevScrollTopRef.current,
413+
scrollViewport.top,
414+
);
415+
const anchor = findViewportRowAnchor(
416+
previousFiles,
417+
previousSectionMetrics,
418+
previousScrollTop,
419+
);
420+
if (anchor) {
421+
const nextTop = resolveViewportRowAnchorTop(files, sectionMetrics, anchor);
422+
const restoreViewportAnchor = () => {
423+
scrollRef.current?.scrollTo(nextTop);
424+
};
425+
426+
restoreViewportAnchor();
427+
suppressNextSelectionAutoScrollRef.current = true;
428+
const retryDelays = [0, 16, 48];
429+
const timeouts = retryDelays.map((delay) => setTimeout(restoreViewportAnchor, delay));
430+
431+
previousWrapLinesRef.current = wrapLines;
432+
previousSectionMetricsRef.current = sectionMetrics;
433+
previousFilesRef.current = files;
434+
435+
return () => {
436+
timeouts.forEach((timeout) => clearTimeout(timeout));
437+
};
438+
}
439+
}
440+
441+
previousWrapLinesRef.current = wrapLines;
442+
previousSectionMetricsRef.current = sectionMetrics;
443+
previousFilesRef.current = files;
444+
}, [files, scrollRef, scrollViewport.top, sectionMetrics, wrapLines, wrapToggleScrollTop]);
445+
446+
useLayoutEffect(() => {
447+
if (suppressNextSelectionAutoScrollRef.current) {
448+
suppressNextSelectionAutoScrollRef.current = false;
449+
return;
450+
}
451+
309452
if (!selectedAnchorId) {
310453
return;
311454
}
@@ -340,7 +483,6 @@ export function DiffPane({
340483
selectedAnchorId,
341484
selectedEstimatedScrollTop,
342485
visibleAgentNotesByFile.size,
343-
wrapLines,
344486
]);
345487

346488
// Configure scroll step size to scroll exactly 1 line per step
@@ -379,7 +521,10 @@ export function DiffPane({
379521
verticalScrollbarOptions={{ visible: false }}
380522
horizontalScrollbarOptions={{ visible: false }}
381523
>
382-
<box style={{ width: "100%", flexDirection: "column", overflow: "visible" }}>
524+
<box
525+
key={`diff-content:${layout}:${wrapLines ? "wrap" : "nowrap"}:${width}`}
526+
style={{ width: "100%", flexDirection: "column", overflow: "visible" }}
527+
>
383528
{files.map((file, index) => {
384529
const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true;
385530
const shouldPrefetchVisibleHighlight =

0 commit comments

Comments
 (0)