Skip to content

Commit b3bb1b0

Browse files
aldevvbenvinegar
andauthored
fix(ui): use full agent-note set for section geometry measurement (#243)
Co-authored-by: Ben Vinegar <ben@benv.ca>
1 parent cac04aa commit b3bb1b0

2 files changed

Lines changed: 69 additions & 4 deletions

File tree

src/ui/components/panes/DiffPane.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,18 @@ export function DiffPane({
442442
return next;
443443
}, [allAgentNotesByFile, selectedFileId, showAgentNotes, visibleViewportFileIds]);
444444

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

@@ -455,21 +462,21 @@ export function DiffPane({
455462
layout,
456463
showHunkHeaders,
457464
theme,
458-
visibleNotes,
465+
notes,
459466
diffContentWidth,
460467
showLineNumbers,
461468
wrapLines,
462469
);
463470
}),
464471
[
472+
allAgentNotesByFile,
465473
baseSectionGeometry,
466474
diffContentWidth,
467475
files,
468476
layout,
469477
showHunkHeaders,
470478
showLineNumbers,
471479
theme,
472-
visibleAgentNotesByFile,
473480
wrapLines,
474481
],
475482
);

src/ui/components/ui-components.test.tsx

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,64 @@ describe("UI components", () => {
856856
}
857857
});
858858

859+
test("DiffPane keeps bottom scroll stable when offscreen agent notes are windowed out", async () => {
860+
const theme = resolveTheme("midnight", null);
861+
const firstFile = createTallDiffFile("first", "first.ts", 18);
862+
firstFile.agent = {
863+
path: firstFile.path,
864+
summary: "first.ts note",
865+
annotations: [
866+
{
867+
newRange: [2, 2],
868+
summary: "Offscreen note should still reserve geometry at EOF.",
869+
rationale:
870+
"If measurement drops this note after first.ts leaves the viewport, max scroll shrinks.",
871+
},
872+
],
873+
};
874+
const files = [firstFile, createTallDiffFile("last", "last.ts", 24)];
875+
const scrollRef = createRef<ScrollBoxRenderable>();
876+
const props = createDiffPaneProps(files, theme, {
877+
diffContentWidth: 88,
878+
headerLabelWidth: 48,
879+
headerStatsWidth: 16,
880+
scrollRef,
881+
selectedFileId: undefined,
882+
separatorWidth: 84,
883+
showAgentNotes: true,
884+
width: 92,
885+
});
886+
const setup = await testRender(<DiffPane {...props} />, {
887+
width: 96,
888+
height: 10,
889+
});
890+
891+
try {
892+
await settleDiffPane(setup);
893+
894+
let bottomScrollTop = 0;
895+
await act(async () => {
896+
scrollRef.current?.scrollTo(1_000_000);
897+
bottomScrollTop = scrollRef.current?.scrollTop ?? 0;
898+
});
899+
expect(bottomScrollTop).toBeGreaterThan(0);
900+
901+
await settleDiffPane(setup);
902+
expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop);
903+
904+
await act(async () => {
905+
scrollRef.current?.scrollTo(bottomScrollTop + 1);
906+
});
907+
await settleDiffPane(setup);
908+
909+
expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop);
910+
} finally {
911+
await act(async () => {
912+
setup.renderer.destroy();
913+
});
914+
}
915+
});
916+
859917
test("DiffPane lets manual scrolling move away from a bottom-clamped file-top alignment", async () => {
860918
const theme = resolveTheme("midnight", null);
861919
const files = [

0 commit comments

Comments
 (0)