From 451863718408bd838cd61e5c5cca9c0668676e9b Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 6 Apr 2026 14:33:04 -0400 Subject: [PATCH 1/5] feat: sync active hunk to mouse scroll position --- src/ui/App.tsx | 1 + src/ui/AppHost.interactions.test.tsx | 118 ++++++++++++++++++++++++- src/ui/components/panes/DiffPane.tsx | 66 ++++++++++++++ src/ui/lib/viewportSelection.test.ts | 92 ++++++++++++++++++++ src/ui/lib/viewportSelection.ts | 123 +++++++++++++++++++++++++++ 5 files changed, 398 insertions(+), 2 deletions(-) create mode 100644 src/ui/lib/viewportSelection.test.ts create mode 100644 src/ui/lib/viewportSelection.ts diff --git a/src/ui/App.tsx b/src/ui/App.tsx index a28defef..c4bc27d3 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -657,6 +657,7 @@ export function App({ width={diffPaneWidth} onOpenAgentNotesAtHunk={openAgentNotesAtHunk} onSelectFile={jumpToFile} + onViewportCenteredHunkChange={review.selectHunk} /> diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 5e2f6558..768019fc 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -5,7 +5,11 @@ import { describe, expect, mock, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; import { act } from "react"; import type { HunkHostClient } from "../mcp/client"; -import type { HunkSessionRegistration, SessionServerMessage } from "../mcp/types"; +import type { + HunkSessionRegistration, + HunkSessionSnapshot, + SessionServerMessage, +} from "../mcp/types"; import type { AppBootstrap, LayoutMode } from "../core/types"; import { createTestGitAppBootstrap } from "../../test/helpers/app-bootstrap"; import { createTestDiffFile as buildTestDiffFile, lines } from "../../test/helpers/diff-helpers"; @@ -41,6 +45,7 @@ function createMockHostClient() { type Bridge = Parameters[0]; let bridge: Bridge = null; + let latestSnapshot: HunkSessionSnapshot | null = null; const registration: HunkSessionRegistration = { sessionId: "session-1", pid: process.pid, @@ -59,9 +64,12 @@ function createMockHostClient() { setBridge: (nextBridge: Bridge) => { bridge = nextBridge; }, - updateSnapshot: () => {}, + updateSnapshot: (snapshot: HunkSessionSnapshot) => { + latestSnapshot = snapshot; + }, } as unknown as HunkHostClient, getBridge: () => bridge, + getLatestSnapshot: () => latestSnapshot, navigateToHunk: async ( input: Extract["input"], ) => { @@ -226,6 +234,40 @@ function createTwoFileHunkBootstrap(): AppBootstrap { }); } +function createMouseScrollSelectionBootstrap(): AppBootstrap { + const firstBeforeLines = createNumberedAssignmentLines(1, 12); + const secondBeforeLines = Array.from( + { length: 90 }, + (_, index) => `export const line${String(index + 13).padStart(2, "0")} = ${index + 13};`, + ); + const secondAfterLines = [...secondBeforeLines]; + + secondAfterLines[0] = "export const line13 = 1300;"; + secondAfterLines[59] = "export const line72 = 7200;"; + secondAfterLines[60] = "export const line73 = 7300;"; + secondAfterLines[61] = "export const line74 = 7400;"; + + return createTestGitAppBootstrap({ + changesetId: "changeset:mouse-scroll-selection", + files: [ + createTestDiffFile( + "first", + "first.ts", + lines(...firstBeforeLines), + lines("export const line01 = 101;", ...createNumberedAssignmentLines(2, 11)), + true, + ), + createTestDiffFile( + "second", + "second.ts", + lines(...secondBeforeLines), + lines(...secondAfterLines), + true, + ), + ], + }); +} + function createCollapsedTopBootstrap(): AppBootstrap { const beforeLines = Array.from( { length: 400 }, @@ -293,6 +335,29 @@ async function waitForFrame( return frame; } +async function waitForSnapshot( + setup: Awaited>, + getSnapshot: () => HunkSessionSnapshot | null, + predicate: (snapshot: HunkSessionSnapshot) => boolean, + attempts = 8, +) { + let snapshot = getSnapshot(); + + for (let attempt = 0; attempt < attempts; attempt += 1) { + if (snapshot && predicate(snapshot)) { + return snapshot; + } + + await act(async () => { + await Bun.sleep(30); + await setup.renderOnce(); + }); + snapshot = getSnapshot(); + } + + return snapshot; +} + function firstVisibleAddedLine(frame: string) { return frame.match(/line\d{2} = 1\d{2}/)?.[0] ?? null; } @@ -1436,6 +1501,55 @@ describe("App interactions", () => { } }); + test("mouse wheel scrolling updates the active file and hunk to the viewport center", async () => { + const { getLatestSnapshot, hostClient } = createMockHostClient(); + const setup = await testRender( + , + { + width: 220, + height: 12, + }, + ); + + try { + await flush(setup); + + expect(getLatestSnapshot()).toMatchObject({ + selectedFilePath: "first.ts", + selectedHunkIndex: 0, + }); + + let snapshot = getLatestSnapshot(); + for (let index = 0; index < 24; index += 1) { + await act(async () => { + await setup.mockMouse.scroll(120, 7, "down"); + }); + await flush(setup); + + snapshot = await waitForSnapshot( + setup, + getLatestSnapshot, + (currentSnapshot) => + currentSnapshot.selectedFilePath === "second.ts" && + currentSnapshot.selectedHunkIndex === 1, + 4, + ); + if (snapshot?.selectedFilePath === "second.ts" && snapshot.selectedHunkIndex === 1) { + break; + } + } + + expect(snapshot).toMatchObject({ + selectedFilePath: "second.ts", + selectedHunkIndex: 1, + }); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("clicking a sidebar file makes that file own the top of the review pane", async () => { const setup = await testRender(, { width: 220, diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 04bcdbd8..703aacb2 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -24,6 +24,7 @@ import { shouldRenderInStreamFileHeader, } from "../../lib/fileSectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; +import { findViewportCenteredHunkTarget } from "../../lib/viewportSelection"; import type { AppTheme } from "../../themes"; import { DiffSection } from "./DiffSection"; import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; @@ -148,6 +149,7 @@ export function DiffPane({ width, onOpenAgentNotesAtHunk, onSelectFile, + onViewportCenteredHunkChange, }: { diffContentWidth: number; files: DiffFile[]; @@ -170,6 +172,7 @@ export function DiffPane({ width: number; onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; onSelectFile: (fileId: string) => void; + onViewportCenteredHunkChange?: (fileId: string, hunkIndex: number) => void; }) { const renderer = useRenderer(); const [prefetchAnchorKey, setPrefetchAnchorKey] = useState(null); @@ -250,6 +253,27 @@ export function DiffPane({ const previousSelectedFileTopAlignRequestIdRef = useRef(selectedFileTopAlignRequestId); const suppressNextSelectionAutoScrollRef = useRef(false); const pendingFileTopAlignFileIdRef = useRef(null); + const mouseScrollSelectionSyncActiveRef = useRef(false); + const mouseScrollSelectionSyncTimeoutRef = useRef | null>(null); + + const armMouseScrollSelectionSync = useCallback(() => { + mouseScrollSelectionSyncActiveRef.current = true; + if (mouseScrollSelectionSyncTimeoutRef.current) { + clearTimeout(mouseScrollSelectionSyncTimeoutRef.current); + } + mouseScrollSelectionSyncTimeoutRef.current = setTimeout(() => { + mouseScrollSelectionSyncActiveRef.current = false; + mouseScrollSelectionSyncTimeoutRef.current = null; + }, 120); + }, []); + + useEffect(() => { + return () => { + if (mouseScrollSelectionSyncTimeoutRef.current) { + clearTimeout(mouseScrollSelectionSyncTimeoutRef.current); + } + }; + }, []); useEffect(() => { const scrollBox = scrollRef.current; @@ -396,6 +420,47 @@ export function DiffPane({ // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; + useLayoutEffect(() => { + if ( + !onViewportCenteredHunkChange || + !mouseScrollSelectionSyncActiveRef.current || + files.length === 0 || + scrollViewport.height <= 0 + ) { + return; + } + + const centeredTarget = findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop: scrollViewport.top, + viewportHeight: scrollViewport.height, + }); + if (!centeredTarget) { + return; + } + + if ( + centeredTarget.fileId === selectedFileId && + centeredTarget.hunkIndex === selectedHunkIndex + ) { + return; + } + + suppressNextSelectionAutoScrollRef.current = true; + onViewportCenteredHunkChange(centeredTarget.fileId, centeredTarget.hunkIndex); + }, [ + fileSectionLayouts, + files, + onViewportCenteredHunkChange, + scrollViewport.height, + scrollViewport.top, + sectionGeometry, + selectedFileId, + selectedHunkIndex, + ]); + const pinnedHeaderFile = useMemo(() => { if (files.length === 0) { return null; @@ -822,6 +887,7 @@ export function DiffPane({ scrollY={true} viewportCulling={true} focused={pagerMode} + onMouseScroll={armMouseScrollSelectionSync} rootOptions={{ backgroundColor: theme.panel }} wrapperOptions={{ backgroundColor: theme.panel }} viewportOptions={{ backgroundColor: theme.panel }} diff --git a/src/ui/lib/viewportSelection.test.ts b/src/ui/lib/viewportSelection.test.ts new file mode 100644 index 00000000..c0b6a731 --- /dev/null +++ b/src/ui/lib/viewportSelection.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, test } from "bun:test"; +import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers"; +import { measureDiffSectionGeometry } from "./diffSectionGeometry"; +import { buildFileSectionLayouts, buildInStreamFileHeaderHeights } from "./fileSectionLayout"; +import { findViewportCenteredHunkTarget } from "./viewportSelection"; +import { resolveTheme } from "../themes"; + +function createWideTwoHunkFile(id: string, path: string, start = 1) { + const beforeLines = Array.from( + { length: 80 }, + (_, index) => `export const line${start + index} = ${start + index};`, + ); + const afterLines = [...beforeLines]; + + afterLines[0] = `export const line${start} = ${start + 1000};`; + afterLines[59] = `export const line${start + 59} = ${start + 5900};`; + + return createTestDiffFile({ + after: lines(...afterLines), + before: lines(...beforeLines), + context: 3, + id, + path, + }); +} + +function scrollTopForCenter(centerOffset: number, viewportHeight: number) { + return Math.max(0, centerOffset - Math.max(0, Math.floor((viewportHeight - 1) / 2))); +} + +describe("findViewportCenteredHunkTarget", () => { + const theme = resolveTheme("midnight", null); + + test("switches the active file when the viewport center enters a later file", () => { + const firstFile = createTestDiffFile({ + after: "export const alpha = 2;\n", + before: "export const alpha = 1;\n", + id: "first", + path: "first.ts", + }); + const secondFile = createWideTwoHunkFile("second", "second.ts", 100); + const files = [firstFile, secondFile]; + const sectionGeometry = files.map((file) => + measureDiffSectionGeometry(file, "split", true, theme, [], 160, true, false), + ); + const fileSectionLayouts = buildFileSectionLayouts( + files, + sectionGeometry.map((geometry) => geometry.bodyHeight), + buildInStreamFileHeaderHeights(files), + ); + const viewportHeight = 7; + const secondFileFirstHunkTop = + fileSectionLayouts[1]!.bodyTop + sectionGeometry[1]!.hunkBounds.get(0)!.top; + + expect( + findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop: scrollTopForCenter(secondFileFirstHunkTop, viewportHeight), + viewportHeight, + }), + ).toEqual({ fileId: "second", hunkIndex: 0 }); + }); + + test("picks the nearest hunk when the viewport center lands in a collapsed gap", () => { + const file = createWideTwoHunkFile("gap", "gap.ts"); + const geometry = measureDiffSectionGeometry(file, "split", true, theme, [], 160, true, false); + const viewportHeight = 7; + const fileSectionLayouts = buildFileSectionLayouts( + [file], + [geometry.bodyHeight], + buildInStreamFileHeaderHeights([file]), + ); + const firstHunk = geometry.hunkBounds.get(0)!; + const secondHunk = geometry.hunkBounds.get(1)!; + const centeredGapOffset = Math.max(firstHunk.top + firstHunk.height, secondHunk.top - 1); + + expect( + findViewportCenteredHunkTarget({ + files: [file], + fileSectionLayouts, + sectionGeometry: [geometry], + scrollTop: scrollTopForCenter( + fileSectionLayouts[0]!.bodyTop + centeredGapOffset, + viewportHeight, + ), + viewportHeight, + }), + ).toEqual({ fileId: "gap", hunkIndex: 1 }); + }); +}); diff --git a/src/ui/lib/viewportSelection.ts b/src/ui/lib/viewportSelection.ts new file mode 100644 index 00000000..86a88812 --- /dev/null +++ b/src/ui/lib/viewportSelection.ts @@ -0,0 +1,123 @@ +import type { DiffFile } from "../../core/types"; +import type { DiffSectionGeometry } from "./diffSectionGeometry"; +import type { FileSectionLayout } from "./fileSectionLayout"; + +export interface ViewportCenteredHunkTarget { + fileId: string; + hunkIndex: number; +} + +/** Find the file section covering one absolute review-stream row. */ +function findFileSectionAtOffset(fileSectionLayouts: FileSectionLayout[], offset: number) { + if (fileSectionLayouts.length === 0) { + return null; + } + + const firstSection = fileSectionLayouts[0]!; + const lastSection = fileSectionLayouts[fileSectionLayouts.length - 1]!; + + if (offset <= firstSection.sectionTop) { + return firstSection; + } + + if (offset >= lastSection.sectionBottom) { + return lastSection; + } + + let low = 0; + let high = fileSectionLayouts.length - 1; + + while (low <= high) { + const mid = (low + high) >>> 1; + const layout = fileSectionLayouts[mid]!; + + if (offset < layout.sectionTop) { + high = mid - 1; + } else if (offset >= layout.sectionBottom) { + low = mid + 1; + } else { + return layout; + } + } + + return lastSection; +} + +/** Pick the hunk nearest one vertical offset within a file body. */ +function findNearestHunkIndexAtBodyOffset( + sectionGeometry: DiffSectionGeometry | undefined, + bodyOffset: number, + hunkCount: number, +) { + if (!sectionGeometry || hunkCount <= 1 || sectionGeometry.hunkBounds.size === 0) { + return 0; + } + + let nearestHunkIndex = 0; + let nearestDistance = Number.POSITIVE_INFINITY; + + for (let hunkIndex = 0; hunkIndex < hunkCount; hunkIndex += 1) { + const hunkBounds = sectionGeometry.hunkBounds.get(hunkIndex); + if (!hunkBounds) { + continue; + } + + const hunkBottom = hunkBounds.top + hunkBounds.height - 1; + if (bodyOffset >= hunkBounds.top && bodyOffset <= hunkBottom) { + return hunkIndex; + } + + const distance = + bodyOffset < hunkBounds.top ? hunkBounds.top - bodyOffset : bodyOffset - hunkBottom; + + // Favor the later hunk on exact ties so ownership hands off when the midpoint reaches center. + if ( + distance < nearestDistance || + (distance === nearestDistance && hunkIndex > nearestHunkIndex) + ) { + nearestDistance = distance; + nearestHunkIndex = hunkIndex; + } + } + + return nearestHunkIndex; +} + +/** Resolve the file and hunk nearest the current review viewport center. */ +export function findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop, + viewportHeight, +}: { + files: DiffFile[]; + fileSectionLayouts: FileSectionLayout[]; + sectionGeometry: DiffSectionGeometry[]; + scrollTop: number; + viewportHeight: number; +}): ViewportCenteredHunkTarget | null { + if (files.length === 0 || fileSectionLayouts.length === 0) { + return null; + } + + const centerOffset = Math.max(0, scrollTop + Math.max(0, Math.floor((viewportHeight - 1) / 2))); + const centeredSection = findFileSectionAtOffset(fileSectionLayouts, centerOffset); + if (!centeredSection) { + return null; + } + + const centeredFile = files[centeredSection.sectionIndex]; + if (!centeredFile) { + return null; + } + + return { + fileId: centeredFile.id, + hunkIndex: findNearestHunkIndexAtBodyOffset( + sectionGeometry[centeredSection.sectionIndex], + centerOffset - centeredSection.bodyTop, + centeredFile.metadata.hunks.length, + ), + }; +} From cea8c7c34385e875cd3e5e42d614a756c8dbbb47 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 6 Apr 2026 15:04:09 -0400 Subject: [PATCH 2/5] perf: prefetch diff highlighting before viewport entry --- src/ui/components/panes/DiffPane.tsx | 83 +++++++++----- src/ui/components/panes/DiffSection.tsx | 3 - src/ui/components/ui-components.test.tsx | 82 +++++++++++++ src/ui/diff/PierreDiffView.tsx | 3 - src/ui/diff/useHighlightedDiff.ts | 139 ++++++++++------------- 5 files changed, 196 insertions(+), 114 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 703aacb2..9d9a739b 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -30,6 +30,7 @@ import { DiffSection } from "./DiffSection"; import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; +import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; @@ -175,16 +176,9 @@ export function DiffPane({ onViewportCenteredHunkChange?: (fileId: string, hunkIndex: number) => void; }) { const renderer = useRenderer(); - const [prefetchAnchorKey, setPrefetchAnchorKey] = useState(null); - const selectedHighlightKey = selectedFileId ? `${theme.appearance}:${selectedFileId}` : null; - useEffect(() => { - setPrefetchAnchorKey(null); - }, [selectedHighlightKey]); - - // Hold background prefetches until the currently selected file has painted once. const adjacentPrefetchFileIds = useMemo(() => { - if (!selectedHighlightKey || prefetchAnchorKey !== selectedHighlightKey || !selectedFileId) { + if (!selectedFileId) { return new Set(); } @@ -206,15 +200,7 @@ export function DiffPane({ } return next; - }, [files, prefetchAnchorKey, selectedFileId, selectedHighlightKey]); - - const handleSelectedHighlightReady = useCallback(() => { - if (!selectedHighlightKey) { - return; - } - - setPrefetchAnchorKey((current) => current ?? selectedHighlightKey); - }, [selectedHighlightKey]); + }, [files, selectedFileId]); const allAgentNotesByFile = useMemo(() => { const next = new Map(); @@ -416,6 +402,51 @@ export function DiffPane({ [estimatedBodyHeights, files, sectionHeaderHeights], ); const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; + + const highlightPrefetchFileIds = useMemo(() => { + const next = new Set(adjacentPrefetchFileIds); + + if (selectedFileId) { + next.add(selectedFileId); + } + + const viewportHeight = Math.max(1, scrollViewport.height); + const prefetchRows = Math.max(24, viewportHeight * 3); + const minPrefetchY = Math.max(0, scrollViewport.top - prefetchRows); + const maxPrefetchY = scrollViewport.top + scrollViewport.height + prefetchRows; + + for (const layout of fileSectionLayouts) { + if (layout.sectionBottom >= minPrefetchY && layout.sectionTop <= maxPrefetchY) { + next.add(layout.fileId); + } + } + + return next; + }, [ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollViewport.height, + scrollViewport.top, + selectedFileId, + ]); + + useEffect(() => { + if (files.length === 0) { + return; + } + + for (const file of files) { + if (!highlightPrefetchFileIds.has(file.id)) { + continue; + } + + void prefetchHighlightedDiff({ + file, + appearance: theme.appearance, + }); + } + }, [files, highlightPrefetchFileIds, theme.appearance]); + // Read the live scroll box position during render so pinned-header ownership flips // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; @@ -722,9 +753,10 @@ export function DiffPane({ useLayoutEffect(() => { const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; - if (suppressNextSelectionAutoScrollRef.current) { + if (mouseScrollSelectionSyncActiveRef.current || suppressNextSelectionAutoScrollRef.current) { suppressNextSelectionAutoScrollRef.current = false; - // Consume this selection transition so the next render does not re-center the selected hunk. + // While the mouse wheel owns the viewport, selection should follow passively instead of + // snapping the review stream back toward the newly selected hunk. prevSelectedAnchorIdRef.current = selectedAnchorId; prevPinnedHeaderFileIdRef.current = pinnedHeaderFileId; pendingSelectionSettleRef.current = false; @@ -903,10 +935,6 @@ export function DiffPane({ > {files.map((file, index) => { const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true; - const shouldPrefetchVisibleHighlight = - Boolean(selectedHighlightKey) && - prefetchAnchorKey === selectedHighlightKey && - visibleViewportFileIds.has(file.id); // Windowing keeps offscreen files cheap: render placeholders with identical // section geometry so scroll math and pinned-header ownership stay stable. @@ -935,14 +963,7 @@ export function DiffPane({ headerStatsWidth={headerStatsWidth} layout={layout} selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1} - shouldLoadHighlight={ - file.id === selectedFileId || - adjacentPrefetchFileIds.has(file.id) || - shouldPrefetchVisibleHighlight - } - onHighlightReady={ - file.id === selectedFileId ? handleSelectedHighlightReady : undefined - } + shouldLoadHighlight={highlightPrefetchFileIds.has(file.id)} separatorWidth={separatorWidth} showHeader={shouldRenderInStreamFileHeader(index)} showSeparator={index > 0} diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index b6aceeae..1fa2fd6f 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -14,7 +14,6 @@ interface DiffSectionProps { layout: Exclude; selectedHunkIndex: number; shouldLoadHighlight: boolean; - onHighlightReady?: () => void; separatorWidth: number; showLineNumbers: boolean; showHunkHeaders: boolean; @@ -36,7 +35,6 @@ function DiffSectionComponent({ layout, selectedHunkIndex, shouldLoadHighlight, - onHighlightReady, separatorWidth, showLineNumbers, showHunkHeaders, @@ -96,7 +94,6 @@ function DiffSectionComponent({ annotatedHunkIndices={annotatedHunkIndices} visibleAgentNotes={visibleAgentNotes} onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk} - onHighlightReady={onHighlightReady} selectedHunkIndex={selectedHunkIndex} shouldLoadHighlight={shouldLoadHighlight} // The parent review stream owns scrolling across files. diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index c46cb8d5..3b5448ea 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -65,6 +65,30 @@ function createWindowingFiles(count: number) { ); } +function createHighlightPrefetchWindowFiles() { + return Array.from({ length: 4 }, (_, index) => { + const marker = `prefetchMarker${index + 1}`; + const before = lines( + `export const ${marker} = ${index + 1};`, + ...Array.from( + { length: 8 }, + (_, lineIndex) => + `export function keep${index + 1}_${lineIndex}(value: number) { return value + ${lineIndex}; }`, + ), + ); + const after = lines( + `export const ${marker} = ${index + 100};`, + ...Array.from( + { length: 8 }, + (_, lineIndex) => + `export function keep${index + 1}_${lineIndex}(value: number) { return value * ${lineIndex + 2}; }`, + ), + ); + + return createTestDiffFile(`prefetch-${index + 1}`, `prefetch-${index + 1}.ts`, before, after); + }); +} + function createMultiHunkDiffFile(id: string, path: string) { const before = lines( "export const line1 = 1;", @@ -1695,6 +1719,64 @@ describe("UI components", () => { } }); + test("DiffPane prefetches highlight data for files approaching the viewport before they mount", async () => { + const files = createHighlightPrefetchWindowFiles(); + const theme = resolveTheme("midnight", null); + const setup = await testRender( + , + { width: 100, height: 10 }, + ); + const thirdFileCheck = await testRender( + , + { width: 184, height: 10 }, + ); + + try { + await settleDiffPane(setup); + + const initialFrame = setup.captureCharFrame(); + expect(initialFrame).not.toContain("prefetch-3.ts"); + + let prefetched = false; + for (let iteration = 0; iteration < 400; iteration += 1) { + await act(async () => { + await setup.renderOnce(); + await thirdFileCheck.renderOnce(); + await Bun.sleep(0); + await setup.renderOnce(); + await thirdFileCheck.renderOnce(); + await Bun.sleep(0); + }); + + if (frameHasHighlightedMarker(thirdFileCheck.captureSpans(), "prefetchMarker3")) { + prefetched = true; + break; + } + } + + expect(prefetched).toBe(true); + } finally { + await act(async () => { + thirdFileCheck.renderer.destroy(); + setup.renderer.destroy(); + }); + } + }); + test("App renders the menu bar, multi-file stream, and AI badges", async () => { const bootstrap = createBootstrap(); const frame = await captureFrame(, 280, 24); diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 3e4b408d..e3f40e26 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -19,7 +19,6 @@ export function PierreDiffView({ file, layout, onOpenAgentNotesAtHunk, - onHighlightReady, showLineNumbers = true, showHunkHeaders = true, wrapLines = false, @@ -34,7 +33,6 @@ export function PierreDiffView({ file: DiffFile | undefined; layout: Exclude; onOpenAgentNotesAtHunk?: (hunkIndex: number) => void; - onHighlightReady?: () => void; showLineNumbers?: boolean; showHunkHeaders?: boolean; wrapLines?: boolean; @@ -48,7 +46,6 @@ export function PierreDiffView({ const resolvedHighlighted = useHighlightedDiff({ file, appearance: theme.appearance, - onHighlightReady, shouldLoadHighlight, }); diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index c0230a14..4516dbd6 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; +import { useLayoutEffect, useState } from "react"; import type { DiffFile } from "../../core/types"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./pierre"; @@ -43,49 +43,73 @@ function commitHighlightResult( if (SHARED_HIGHLIGHT_PROMISES.get(cacheKey) !== promise) { return false; } + SHARED_HIGHLIGHT_PROMISES.delete(cacheKey); SHARED_HIGHLIGHTED_DIFF_CACHE.set(cacheKey, result); enforceCacheLimit(); return true; } +/** Start one shared highlight request unless the cache or an in-flight promise already has it. */ +function ensureHighlightedDiffLoaded( + file: DiffFile, + appearance: "light" | "dark", + cacheKey = buildCacheKey(appearance, file), +) { + const cached = SHARED_HIGHLIGHTED_DIFF_CACHE.get(cacheKey); + if (cached) { + return Promise.resolve(cached); + } + + const existing = SHARED_HIGHLIGHT_PROMISES.get(cacheKey); + if (existing) { + return existing; + } + + let pending: Promise; + pending = loadHighlightedDiff(file, appearance) + .then((nextHighlighted) => { + commitHighlightResult(cacheKey, pending, nextHighlighted); + return nextHighlighted; + }) + .catch(() => { + const fallback = { + deletionLines: [], + additionLines: [], + } satisfies HighlightedDiffCode; + commitHighlightResult(cacheKey, pending, fallback); + return fallback; + }); + + SHARED_HIGHLIGHT_PROMISES.set(cacheKey, pending); + return pending; +} + +/** Queue syntax highlighting for one file without mounting its diff rows first. */ +export function prefetchHighlightedDiff({ + file, + appearance, +}: { + file: DiffFile; + appearance: "light" | "dark"; +}) { + return ensureHighlightedDiffLoaded(file, appearance); +} + /** Resolve highlighted diff content with shared caching and background prefetch support. */ export function useHighlightedDiff({ file, appearance, - onHighlightReady, shouldLoadHighlight, }: { file: DiffFile | undefined; appearance: "light" | "dark"; - onHighlightReady?: () => void; shouldLoadHighlight?: boolean; }) { const [highlighted, setHighlighted] = useState(null); const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; - // Selected files load immediately; background prefetch can opt neighboring files in later. - const pendingHighlight = useMemo(() => { - if ( - !shouldLoadHighlight || - !file || - !appearanceCacheKey || - SHARED_HIGHLIGHTED_DIFF_CACHE.has(appearanceCacheKey) - ) { - return null; - } - - const existing = SHARED_HIGHLIGHT_PROMISES.get(appearanceCacheKey); - if (existing) { - return existing; - } - - const pending = loadHighlightedDiff(file, appearance); - SHARED_HIGHLIGHT_PROMISES.set(appearanceCacheKey, pending); - return pending; - }, [appearance, appearanceCacheKey, file, shouldLoadHighlight]); - useLayoutEffect(() => { if (!file || !appearanceCacheKey) { setHighlighted(null); @@ -111,63 +135,24 @@ export function useHighlightedDiff({ let cancelled = false; setHighlighted(null); - // Capture the key and promise reference this effect was started for so the - // resolution callback only writes if it is still the active request. - const effectCacheKey = appearanceCacheKey; - const effectPromise = pendingHighlight; - - effectPromise - ?.then((nextHighlighted) => { - if (cancelled) { - return; - } - - if (commitHighlightResult(effectCacheKey, effectPromise, nextHighlighted)) { - setHighlighted(nextHighlighted); - setHighlightedCacheKey(effectCacheKey); - } - }) - .catch(() => { - if (cancelled) { - return; - } - - const fallback = { - deletionLines: [], - additionLines: [], - } satisfies HighlightedDiffCode; - if (commitHighlightResult(effectCacheKey, effectPromise, fallback)) { - setHighlighted(fallback); - setHighlightedCacheKey(effectCacheKey); - } - }); + ensureHighlightedDiffLoaded(file, appearance, appearanceCacheKey).then((nextHighlighted) => { + if (cancelled) { + return; + } + + setHighlighted(nextHighlighted); + setHighlightedCacheKey(appearanceCacheKey); + }); return () => { cancelled = true; }; - }, [appearanceCacheKey, file, highlightedCacheKey, pendingHighlight, shouldLoadHighlight]); + }, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]); // Prefer cached highlights during render so revisiting a file can paint immediately. - const resolvedHighlighted = - appearanceCacheKey && highlightedCacheKey === appearanceCacheKey - ? highlighted - : appearanceCacheKey - ? (SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null) - : null; - const notifiedHighlightKeyRef = useRef(null); - - useEffect(() => { - if (!onHighlightReady || !appearanceCacheKey || !resolvedHighlighted) { - return; - } - - if (notifiedHighlightKeyRef.current === appearanceCacheKey) { - return; - } - - notifiedHighlightKeyRef.current = appearanceCacheKey; - onHighlightReady(); - }, [appearanceCacheKey, onHighlightReady, resolvedHighlighted]); - - return resolvedHighlighted; + return appearanceCacheKey && highlightedCacheKey === appearanceCacheKey + ? highlighted + : appearanceCacheKey + ? (SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null) + : null; } From a5a7f674551475fef4fa18775cfa4c1344a3e68b Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 6 Apr 2026 15:17:44 -0400 Subject: [PATCH 3/5] refactor: document viewport prefetch and scroll sync --- src/ui/components/panes/DiffPane.tsx | 145 ++++++++++++++++++--------- src/ui/diff/useHighlightedDiff.ts | 33 +++++- 2 files changed, 123 insertions(+), 55 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 9d9a739b..ff20d6ef 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -22,6 +22,7 @@ import { buildInStreamFileHeaderHeights, findHeaderOwningFileSection, shouldRenderInStreamFileHeader, + type FileSectionLayout, } from "../../lib/fileSectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; import { findViewportCenteredHunkTarget } from "../../lib/viewportSelection"; @@ -127,6 +128,73 @@ function resolveViewportRowAnchorTop( return 0; } +/** Keep syntax-highlight warm for the files immediately adjacent to the current selection. */ +function buildAdjacentPrefetchFileIds(files: DiffFile[], selectedFileId?: string) { + if (!selectedFileId) { + return new Set(); + } + + const selectedIndex = files.findIndex((file) => file.id === selectedFileId); + if (selectedIndex < 0) { + return new Set(); + } + + const next = new Set(); + const previousFile = files[selectedIndex - 1]; + const nextFile = files[selectedIndex + 1]; + + if (previousFile) { + next.add(previousFile.id); + } + + if (nextFile) { + next.add(nextFile.id); + } + + return next; +} + +/** + * Start highlight work before files visibly enter the review stream. + * + * We intentionally include three groups: + * - the selected file, so direct navigation always warms the active target + * - adjacent files, so hunk/file navigation does not wait on a cold highlight + * - files within a larger viewport halo, so wheel/track scrolling sees colorized rows already ready + */ +function buildHighlightPrefetchFileIds({ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollTop, + viewportHeight, + selectedFileId, +}: { + adjacentPrefetchFileIds: Set; + fileSectionLayouts: FileSectionLayout[]; + scrollTop: number; + viewportHeight: number; + selectedFileId?: string; +}) { + const next = new Set(adjacentPrefetchFileIds); + + if (selectedFileId) { + next.add(selectedFileId); + } + + const clampedViewportHeight = Math.max(1, viewportHeight); + const prefetchRows = Math.max(24, clampedViewportHeight * 3); + const minPrefetchY = Math.max(0, scrollTop - prefetchRows); + const maxPrefetchY = scrollTop + viewportHeight + prefetchRows; + + for (const layout of fileSectionLayouts) { + if (layout.sectionBottom >= minPrefetchY && layout.sectionTop <= maxPrefetchY) { + next.add(layout.fileId); + } + } + + return next; +} + /** Render the main multi-file review stream. */ export function DiffPane({ diffContentWidth, @@ -177,30 +245,10 @@ export function DiffPane({ }) { const renderer = useRenderer(); - const adjacentPrefetchFileIds = useMemo(() => { - if (!selectedFileId) { - return new Set(); - } - - const selectedIndex = files.findIndex((file) => file.id === selectedFileId); - if (selectedIndex < 0) { - return new Set(); - } - - const next = new Set(); - const previousFile = files[selectedIndex - 1]; - const nextFile = files[selectedIndex + 1]; - - if (previousFile) { - next.add(previousFile.id); - } - - if (nextFile) { - next.add(nextFile.id); - } - - return next; - }, [files, selectedFileId]); + const adjacentPrefetchFileIds = useMemo( + () => buildAdjacentPrefetchFileIds(files, selectedFileId), + [files, selectedFileId], + ); const allAgentNotesByFile = useMemo(() => { const next = new Map(); @@ -261,6 +309,8 @@ export function DiffPane({ }; }, []); + // Mirror the imperative OpenTUI scrollbox state into React state so geometry planning, + // windowing, pinned-header ownership, and prefetching can all read the same viewport snapshot. useEffect(() => { const scrollBox = scrollRef.current; if (!scrollBox) { @@ -403,33 +453,26 @@ export function DiffPane({ ); const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; - const highlightPrefetchFileIds = useMemo(() => { - const next = new Set(adjacentPrefetchFileIds); - - if (selectedFileId) { - next.add(selectedFileId); - } - - const viewportHeight = Math.max(1, scrollViewport.height); - const prefetchRows = Math.max(24, viewportHeight * 3); - const minPrefetchY = Math.max(0, scrollViewport.top - prefetchRows); - const maxPrefetchY = scrollViewport.top + scrollViewport.height + prefetchRows; - - for (const layout of fileSectionLayouts) { - if (layout.sectionBottom >= minPrefetchY && layout.sectionTop <= maxPrefetchY) { - next.add(layout.fileId); - } - } - - return next; - }, [ - adjacentPrefetchFileIds, - fileSectionLayouts, - scrollViewport.height, - scrollViewport.top, - selectedFileId, - ]); + const highlightPrefetchFileIds = useMemo( + () => + buildHighlightPrefetchFileIds({ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollTop: scrollViewport.top, + viewportHeight: scrollViewport.height, + selectedFileId, + }), + [ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollViewport.height, + scrollViewport.top, + selectedFileId, + ], + ); + // Kick off highlight work from viewport planning rather than waiting for the section to mount. + // That avoids the "plain rows first, color later" stutter when a file is about to scroll onscreen. useEffect(() => { if (files.length === 0) { return; @@ -451,6 +494,8 @@ export function DiffPane({ // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; + // While the wheel is actively scrolling, selection follows the viewport center instead of + // driving the viewport. That keeps sidebar state in sync without introducing scroll snap-back. useLayoutEffect(() => { if ( !onViewportCenteredHunkChange || diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index 4516dbd6..e82fcc16 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -96,6 +96,27 @@ export function prefetchHighlightedDiff({ return ensureHighlightedDiffLoaded(file, appearance); } +/** Read the best already-available highlight result without starting async work during render. */ +function resolveHighlightedSnapshot({ + appearanceCacheKey, + highlighted, + highlightedCacheKey, +}: { + appearanceCacheKey: string | null; + highlighted: HighlightedDiffCode | null; + highlightedCacheKey: string | null; +}) { + if (!appearanceCacheKey) { + return null; + } + + if (highlightedCacheKey === appearanceCacheKey) { + return highlighted; + } + + return SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null; +} + /** Resolve highlighted diff content with shared caching and background prefetch support. */ export function useHighlightedDiff({ file, @@ -110,6 +131,8 @@ export function useHighlightedDiff({ const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; + // Use a layout effect so a newly available cached result can replace the plain-text fallback + // before the next diff paint whenever possible. That reduces flash/stutter as files enter view. useLayoutEffect(() => { if (!file || !appearanceCacheKey) { setHighlighted(null); @@ -150,9 +173,9 @@ export function useHighlightedDiff({ }, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]); // Prefer cached highlights during render so revisiting a file can paint immediately. - return appearanceCacheKey && highlightedCacheKey === appearanceCacheKey - ? highlighted - : appearanceCacheKey - ? (SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null) - : null; + return resolveHighlightedSnapshot({ + appearanceCacheKey, + highlighted, + highlightedCacheKey, + }); } From 1795418b27353bef8b00ed1902437857b2f29651 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 6 Apr 2026 15:33:06 -0400 Subject: [PATCH 4/5] refactor: share file section viewport helpers --- src/ui/components/panes/DiffPane.tsx | 17 ++++---- src/ui/lib/fileSectionLayout.test.ts | 58 ++++++++++++++++++++++++++++ src/ui/lib/fileSectionLayout.ts | 53 +++++++++++++++++++++++++ src/ui/lib/viewportSelection.ts | 38 +----------------- 4 files changed, 120 insertions(+), 46 deletions(-) create mode 100644 src/ui/lib/fileSectionLayout.test.ts diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index ff20d6ef..7b33689b 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -20,6 +20,7 @@ import { import { buildFileSectionLayouts, buildInStreamFileHeaderHeights, + collectIntersectingFileSectionIds, findHeaderOwningFileSection, shouldRenderInStreamFileHeader, type FileSectionLayout, @@ -186,10 +187,12 @@ function buildHighlightPrefetchFileIds({ const minPrefetchY = Math.max(0, scrollTop - prefetchRows); const maxPrefetchY = scrollTop + viewportHeight + prefetchRows; - for (const layout of fileSectionLayouts) { - if (layout.sectionBottom >= minPrefetchY && layout.sectionTop <= maxPrefetchY) { - next.add(layout.fileId); - } + for (const fileId of collectIntersectingFileSectionIds( + fileSectionLayouts, + minPrefetchY, + maxPrefetchY, + )) { + next.add(fileId); } return next; @@ -381,11 +384,7 @@ export function DiffPane({ const overscanRows = 8; const minVisibleY = Math.max(0, scrollViewport.top - overscanRows); const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows; - return new Set( - baseFileSectionLayouts - .filter((metric) => metric.sectionBottom >= minVisibleY && metric.sectionTop <= maxVisibleY) - .map((metric) => metric.fileId), - ); + return collectIntersectingFileSectionIds(baseFileSectionLayouts, minVisibleY, maxVisibleY); }, [baseFileSectionLayouts, scrollViewport.height, scrollViewport.top]); const visibleAgentNotesByFile = useMemo(() => { diff --git a/src/ui/lib/fileSectionLayout.test.ts b/src/ui/lib/fileSectionLayout.test.ts new file mode 100644 index 00000000..bd0f0041 --- /dev/null +++ b/src/ui/lib/fileSectionLayout.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from "bun:test"; +import { + collectIntersectingFileSectionIds, + findFileSectionAtOffset, + type FileSectionLayout, +} from "./fileSectionLayout"; + +const layouts: FileSectionLayout[] = [ + { + fileId: "alpha", + sectionIndex: 0, + sectionTop: 0, + headerTop: 0, + bodyTop: 0, + bodyHeight: 5, + sectionBottom: 5, + }, + { + fileId: "beta", + sectionIndex: 1, + sectionTop: 5, + headerTop: 6, + bodyTop: 7, + bodyHeight: 4, + sectionBottom: 11, + }, + { + fileId: "gamma", + sectionIndex: 2, + sectionTop: 11, + headerTop: 12, + bodyTop: 13, + bodyHeight: 6, + sectionBottom: 19, + }, +]; + +describe("fileSectionLayout helpers", () => { + test("findFileSectionAtOffset returns the containing section and clamps past the ends", () => { + expect(findFileSectionAtOffset([], 3)).toBeNull(); + expect(findFileSectionAtOffset(layouts, -5)?.fileId).toBe("alpha"); + expect(findFileSectionAtOffset(layouts, 4)?.fileId).toBe("alpha"); + expect(findFileSectionAtOffset(layouts, 5)?.fileId).toBe("beta"); + expect(findFileSectionAtOffset(layouts, 10)?.fileId).toBe("beta"); + expect(findFileSectionAtOffset(layouts, 11)?.fileId).toBe("gamma"); + expect(findFileSectionAtOffset(layouts, 99)?.fileId).toBe("gamma"); + }); + + test("collectIntersectingFileSectionIds returns every file whose section overlaps the range", () => { + expect(Array.from(collectIntersectingFileSectionIds(layouts, 6, 10))).toEqual(["beta"]); + expect(Array.from(collectIntersectingFileSectionIds(layouts, 4, 12))).toEqual([ + "alpha", + "beta", + "gamma", + ]); + expect(Array.from(collectIntersectingFileSectionIds(layouts, 20, 24))).toEqual([]); + }); +}); diff --git a/src/ui/lib/fileSectionLayout.ts b/src/ui/lib/fileSectionLayout.ts index 622049b7..9452450a 100644 --- a/src/ui/lib/fileSectionLayout.ts +++ b/src/ui/lib/fileSectionLayout.ts @@ -60,6 +60,59 @@ export function buildFileSectionLayouts( return layouts; } +/** Find the file section covering one absolute review-stream row. */ +export function findFileSectionAtOffset(fileSectionLayouts: FileSectionLayout[], offset: number) { + if (fileSectionLayouts.length === 0) { + return null; + } + + const firstSection = fileSectionLayouts[0]!; + const lastSection = fileSectionLayouts[fileSectionLayouts.length - 1]!; + + if (offset <= firstSection.sectionTop) { + return firstSection; + } + + if (offset >= lastSection.sectionBottom) { + return lastSection; + } + + let low = 0; + let high = fileSectionLayouts.length - 1; + + while (low <= high) { + const mid = (low + high) >>> 1; + const layout = fileSectionLayouts[mid]!; + + if (offset < layout.sectionTop) { + high = mid - 1; + } else if (offset >= layout.sectionBottom) { + low = mid + 1; + } else { + return layout; + } + } + + return lastSection; +} + +/** Collect every file section that intersects one absolute review-stream range. */ +export function collectIntersectingFileSectionIds( + fileSectionLayouts: FileSectionLayout[], + minY: number, + maxY: number, +) { + const next = new Set(); + + for (const layout of fileSectionLayouts) { + if (layout.sectionBottom >= minY && layout.sectionTop <= maxY) { + next.add(layout.fileId); + } + } + + return next; +} + /** Return the file section that owns the viewport top, switching at each next header row. */ export function findHeaderOwningFileSection( fileSectionLayouts: FileSectionLayout[], diff --git a/src/ui/lib/viewportSelection.ts b/src/ui/lib/viewportSelection.ts index 86a88812..53f99f6b 100644 --- a/src/ui/lib/viewportSelection.ts +++ b/src/ui/lib/viewportSelection.ts @@ -1,48 +1,12 @@ import type { DiffFile } from "../../core/types"; import type { DiffSectionGeometry } from "./diffSectionGeometry"; -import type { FileSectionLayout } from "./fileSectionLayout"; +import { findFileSectionAtOffset, type FileSectionLayout } from "./fileSectionLayout"; export interface ViewportCenteredHunkTarget { fileId: string; hunkIndex: number; } -/** Find the file section covering one absolute review-stream row. */ -function findFileSectionAtOffset(fileSectionLayouts: FileSectionLayout[], offset: number) { - if (fileSectionLayouts.length === 0) { - return null; - } - - const firstSection = fileSectionLayouts[0]!; - const lastSection = fileSectionLayouts[fileSectionLayouts.length - 1]!; - - if (offset <= firstSection.sectionTop) { - return firstSection; - } - - if (offset >= lastSection.sectionBottom) { - return lastSection; - } - - let low = 0; - let high = fileSectionLayouts.length - 1; - - while (low <= high) { - const mid = (low + high) >>> 1; - const layout = fileSectionLayouts[mid]!; - - if (offset < layout.sectionTop) { - high = mid - 1; - } else if (offset >= layout.sectionBottom) { - low = mid + 1; - } else { - return layout; - } - } - - return lastSection; -} - /** Pick the hunk nearest one vertical offset within a file body. */ function findNearestHunkIndexAtBodyOffset( sectionGeometry: DiffSectionGeometry | undefined, From c361a038d758565ed5cf347f6052aea738c69160 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 6 Apr 2026 15:42:30 -0400 Subject: [PATCH 5/5] fix: combine diff pane wheel handlers --- src/ui/components/panes/DiffPane.tsx | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 80d8dd52..7e16ddcc 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -349,6 +349,27 @@ export function DiffPane({ }, 120); }, []); + /** + * Combine the existing horizontal-wheel handler with viewport-centered selection sync. + * Horizontal gestures should keep their current behavior without changing the active hunk. + */ + const handleDiffPaneMouseScroll = useCallback( + (event: TuiMouseEvent) => { + const direction = event.scroll?.direction; + const isHorizontalGesture = + direction === "left" || + direction === "right" || + (event.modifiers.shift && (direction === "up" || direction === "down")); + + if (!isHorizontalGesture) { + armMouseScrollSelectionSync(); + } + + handleMouseScroll(event); + }, + [armMouseScrollSelectionSync, handleMouseScroll], + ); + useEffect(() => { return () => { if (mouseScrollSelectionSyncTimeoutRef.current) { @@ -1008,14 +1029,13 @@ export function DiffPane({ scrollY={true} viewportCulling={true} focused={pagerMode} - onMouseScroll={armMouseScrollSelectionSync} + onMouseScroll={handleDiffPaneMouseScroll} rootOptions={{ backgroundColor: theme.panel }} wrapperOptions={{ backgroundColor: theme.panel }} viewportOptions={{ backgroundColor: theme.panel }} contentOptions={{ backgroundColor: theme.panel }} verticalScrollbarOptions={{ visible: false }} horizontalScrollbarOptions={{ visible: false }} - onMouseScroll={handleMouseScroll} >