Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/ui/components/panes/DiffPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,18 @@ export function DiffPane({
return next;
}, [allAgentNotesByFile, selectedFileId, showAgentNotes, visibleViewportFileIds]);

// Measure with the *full* set of agent notes per file, not just the visible-viewport set.
// The visible set is correct for rendering (skip painting cards on off-screen files), but
// using it here makes total content height fluctuate with scroll position: as a file with
// notes leaves the viewport, its measurement shrinks back to the no-notes baseline, which
// shrinks `totalContentHeight`, which tightens `clampReviewScrollTop`'s ceiling, which
// snaps the viewport upward by the height of the off-top note rows. Always include notes
// in geometry for stable bottom-edge clamping.
const sectionGeometry = useMemo(
() =>
files.map((file, index) => {
const visibleNotes = visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES;
if (visibleNotes.length === 0) {
const notes = allAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES;
if (notes.length === 0) {
return baseSectionGeometry[index]!;
}

Expand All @@ -428,21 +435,21 @@ export function DiffPane({
layout,
showHunkHeaders,
theme,
visibleNotes,
notes,
diffContentWidth,
showLineNumbers,
wrapLines,
);
}),
[
allAgentNotesByFile,
baseSectionGeometry,
diffContentWidth,
files,
layout,
showHunkHeaders,
showLineNumbers,
theme,
visibleAgentNotesByFile,
wrapLines,
],
);
Expand Down
58 changes: 58 additions & 0 deletions src/ui/components/ui-components.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,64 @@ describe("UI components", () => {
}
});

test("DiffPane keeps bottom scroll stable when offscreen agent notes are windowed out", async () => {
const theme = resolveTheme("midnight", null);
const firstFile = createTallDiffFile("first", "first.ts", 18);
firstFile.agent = {
path: firstFile.path,
summary: "first.ts note",
annotations: [
{
newRange: [2, 2],
summary: "Offscreen note should still reserve geometry at EOF.",
rationale:
"If measurement drops this note after first.ts leaves the viewport, max scroll shrinks.",
},
],
};
const files = [firstFile, createTallDiffFile("last", "last.ts", 24)];
const scrollRef = createRef<ScrollBoxRenderable>();
const props = createDiffPaneProps(files, theme, {
diffContentWidth: 88,
headerLabelWidth: 48,
headerStatsWidth: 16,
scrollRef,
selectedFileId: undefined,
separatorWidth: 84,
showAgentNotes: true,
width: 92,
});
const setup = await testRender(<DiffPane {...props} />, {
width: 96,
height: 10,
});

try {
await settleDiffPane(setup);

let bottomScrollTop = 0;
await act(async () => {
scrollRef.current?.scrollTo(1_000_000);
bottomScrollTop = scrollRef.current?.scrollTop ?? 0;
});
expect(bottomScrollTop).toBeGreaterThan(0);

await settleDiffPane(setup);
expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop);

await act(async () => {
scrollRef.current?.scrollTo(bottomScrollTop + 1);
});
await settleDiffPane(setup);

expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop);
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});

test("DiffPane lets manual scrolling move away from a bottom-clamped file-top alignment", async () => {
const theme = resolveTheme("midnight", null);
const files = [
Expand Down
Loading