diff --git a/.changeset/fix-startup-file-window.md b/.changeset/fix-startup-file-window.md new file mode 100644 index 000000000..c6e6c14e0 --- /dev/null +++ b/.changeset/fix-startup-file-window.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Fill the review stream on first paint instead of leaving it blank until the user scrolls. diff --git a/src/ui/App.tsx b/src/ui/App.tsx index d53a9f600..804f32241 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -296,6 +296,10 @@ export function App({ const layoutToggleScrollTopRef = useRef(null); const cancelCopySelectionRef = useRef<(() => void) | null>(null); const [layoutToggleRequestId, setLayoutToggleRequestId] = useState(0); + const [scrollEdgeRequest, setScrollEdgeRequest] = useState<{ + id: number; + edge: "top" | "bottom"; + }>({ id: 0, edge: "top" }); const [transientNoticeText, setTransientNoticeText] = useState(null); const [layoutMode, setLayoutMode] = useState(bootstrap.initialMode); const [themeId, setThemeId] = useState( @@ -1395,6 +1399,15 @@ export function App({ delta: number, unit: "step" | "viewport" | "content" | "half" = "viewport", ) => { + if (unit === "content") { + if (delta !== 0) { + setScrollEdgeRequest((current) => ({ + id: current.id + 1, + edge: delta > 0 ? "bottom" : "top", + })); + } + return; + } if (unit === "half") { const scrollBox = diffScrollRef.current; if (!scrollBox) return; @@ -2407,6 +2420,7 @@ export function App({ wrapToggleScrollTop={wrapToggleScrollTopRef.current} layoutToggleScrollTop={layoutToggleScrollTopRef.current} layoutToggleRequestId={layoutToggleRequestId} + scrollEdgeRequest={scrollEdgeRequest} selectedFileTopAlignRequestId={review.selectedFileTopAlignRequestId} selectedHunkRevealRequestId={review.selectedHunkRevealRequestId} cursorLine={cursorLine} diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index cf44eb62f..7450abd7f 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -433,6 +433,14 @@ async function flush(setup: Awaited>) { }); } +/** Let initial viewport measurement enable row windowing before testing imperative scroll jumps. */ +async function settleViewportMeasurement(setup: Awaited>) { + await act(async () => { + await Bun.sleep(32); + await setup.renderOnce(); + }); +} + /** Let wrap-toggle renders and follow-up layout retries settle before asserting on the frame. */ async function settleWrapToggle(setup: Awaited>) { await flush(setup); @@ -2462,6 +2470,7 @@ describe("App interactions", () => { try { await flush(setup); + await settleViewportMeasurement(setup); let frame = setup.captureCharFrame(); expect(frame).toContain("line01 = 1001"); @@ -2485,6 +2494,39 @@ describe("App interactions", () => { } }); + test("G supersedes a pending selected-hunk reveal", async () => { + const setup = await testRender( + , + { + width: 120, + height: 16, + }, + ); + + try { + await flush(setup); + await settleViewportMeasurement(setup); + await pressHunkNavigationKey(setup, "]", 1); + + await act(async () => { + await setup.mockInput.pressKey("g", { shift: true }); + }); + await flush(setup); + await act(async () => { + await Bun.sleep(160); + await setup.renderOnce(); + }); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("export const mid = 4;"); + expect(frame).not.toContain("line 021 changed"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("pager mode also supports G and g top/bottom jumps", async () => { const before = Array.from( @@ -2525,6 +2567,7 @@ describe("App interactions", () => { try { await flush(setup); + await settleViewportMeasurement(setup); let frame = setup.captureCharFrame(); expect(frame).toContain("line01 = 1001"); diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 1cc7725bd..728dea0c5 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -51,6 +51,7 @@ import { clampLineCursorToViewport, EMPTY_LINE_CURSORS, firstLineCursorInHunk, + reuseEquivalentLineCursors, type LineCursor, type LineCursorBoundsLookup, } from "../../lib/lineCursors"; @@ -70,7 +71,11 @@ import { } from "../../lib/fileSectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; import { findViewportCenteredHunkTarget } from "../../lib/viewportSelection"; -import { VIEWPORT_READ_COALESCE_MS } from "../../lib/viewportTiming"; +import { + estimateInitialRenderViewportHeight, + resolveRenderViewportHeight, + VIEWPORT_READ_COALESCE_MS, +} from "../../lib/viewportTiming"; import { findViewportRowAnchor, resolveViewportRowAnchorTop, @@ -150,11 +155,6 @@ function clampVerticalScrollTop(scrollTop: number, contentHeight: number, viewpo return Math.min(Math.max(0, scrollTop), maxScrollTop); } -/** Estimate render-only viewport bounds before OpenTUI publishes exact scrollbox geometry. */ -function estimateInitialRenderViewportHeight(rendererHeight: number, screenTop: number) { - return Math.max(1, rendererHeight - Math.max(0, screenTop)); -} - /** Resolve one file-relative measured row through its precomputed whole-stream section layout. */ function streamRowBoundsAt( layouts: FileSectionLayout[], @@ -293,6 +293,7 @@ export function DiffPane({ wrapToggleScrollTop, layoutToggleScrollTop = null, layoutToggleRequestId = 0, + scrollEdgeRequest, selectedFileTopAlignRequestId = 0, selectedHunkRevealRequestId, theme, @@ -356,6 +357,7 @@ export function DiffPane({ wrapToggleScrollTop: number | null; layoutToggleScrollTop?: number | null; layoutToggleRequestId?: number; + scrollEdgeRequest?: { id: number; edge: "top" | "bottom" }; selectedFileTopAlignRequestId?: number; selectedHunkRevealRequestId?: number; theme: AppTheme; @@ -650,6 +652,7 @@ export function DiffPane({ const previousFilesRef = useRef(files); const previousLayoutRef = useRef(layout); const previousWrapLinesRef = useRef(wrapLines); + const previousViewportPaneHeightRef = useRef(height); const draftNoteId = draftNote?.id ?? null; const draftNoteFileId = draftNote?.fileId ?? null; const previousDraftNoteIdRef = useRef(draftNoteId); @@ -666,6 +669,30 @@ export function DiffPane({ // is required before passive viewport-follow selection can trigger. const lastViewportSelectionTopRef = useRef(null); const lastViewportRowAnchorRef = useRef(null); + // Track the previous selected anchor to detect actual selection changes. + const prevSelectedAnchorIdRef = useRef(null); + const prevPinnedHeaderFileIdRef = useRef(null); + const pendingSelectionSettleRef = useRef(false); + const pendingSelectionRevealTimeoutsRef = useRef[]>([]); + + /** Clear scheduled selection-reveal retries without changing the resettle policy. */ + const clearPendingSelectionRevealTimers = useCallback(() => { + for (const timeout of pendingSelectionRevealTimeoutsRef.current) { + clearTimeout(timeout); + } + pendingSelectionRevealTimeoutsRef.current = []; + }, []); + + /** Retire selection reveal work once another explicit scroll policy becomes authoritative. */ + const supersedePendingSelectionReveal = useCallback(() => { + clearPendingSelectionRevealTimers(); + pendingSelectionSettleRef.current = false; + }, [clearPendingSelectionRevealTimers]); + + /** Clear any pending "selected file to top" follow-up. */ + const clearPendingFileTopAlign = useCallback(() => { + pendingFileTopAlignFileIdRef.current = null; + }, []); /** Track the currently hover-owned file without making scroll handlers depend on render state. */ const setHoveredFileForRowActions = useCallback((fileId: string) => { @@ -733,14 +760,20 @@ export function DiffPane({ if (!scrollBox) { return; } + const paneHeightChanged = previousViewportPaneHeightRef.current !== height; + previousViewportPaneHeightRef.current = height; let cancelled = false; let scheduled = false; let scheduledViewportRead: ReturnType | null = null; + let lastReadTop = scrollBox.scrollTop ?? 0; + let lastReadHeight = scrollBox.viewport.height ?? 0; const readViewport = () => { const nextTop = scrollBox.scrollTop ?? 0; const nextHeight = scrollBox.viewport.height ?? 0; + lastReadTop = nextTop; + lastReadHeight = nextHeight; // The first viewport read is a baseline snapshot, not scroll input. The scroll box may retain // a non-zero top across remounts, so do not treat that retained position as a rapid burst. @@ -770,15 +803,15 @@ export function DiffPane({ ); }; - // OpenTUI emits `change` synchronously from inside its own slider sync, and other - // useLayoutEffects in this pane scroll the box from inside React's commit phase. - // Calling setScrollViewport directly from the listener can run setState while React - // is already committing — which downstream layout effects can amplify into a render - // loop and trip React's max-update-depth guard. Coalesce listener events into one - // timer-deferred read so rapid wheel/key bursts collapse into bounded React updates instead of - // turning every native scroll delta into a full review-stream render. Wrapped views use a - // half-frame interval to reduce blank-band latency; nowrap retains one frame. + // OpenTUI emits viewport events from its own layout and slider work. Keep React state updates + // timer-deferred so wheel/key bursts collapse into bounded review-stream renders. const handleViewportChange = () => { + if ( + (scrollBox.scrollTop ?? 0) === lastReadTop && + (scrollBox.viewport.height ?? 0) === lastReadHeight + ) { + return; + } if (scheduled) { return; } @@ -801,10 +834,26 @@ export function DiffPane({ ); }; + // Wait for one real Yoga height change only when geometry is still unknown or the planned pane + // height changed. Leaving this armed after a successful read feeds later content relayouts back + // into React even though the viewport height is already authoritative. + const handleViewportResize = () => { + if ((scrollBox.viewport.height ?? 0) === lastReadHeight) { + return; + } + scrollBox.viewport.off("resize", handleViewportResize); + queueMicrotask(() => { + if (!cancelled) { + readViewport(); + } + }); + }; + readViewport(); scrollBox.verticalScrollBar.on("change", handleViewportChange); - scrollBox.viewport.on("layout-changed", handleViewportChange); - scrollBox.viewport.on("resized", handleViewportChange); + if (lastReadHeight <= 0 || paneHeightChanged) { + scrollBox.viewport.on("resize", handleViewportResize); + } return () => { cancelled = true; @@ -812,10 +861,16 @@ export function DiffPane({ clearTimeout(scheduledViewportRead); } scrollBox.verticalScrollBar.off("change", handleViewportChange); - scrollBox.viewport.off("layout-changed", handleViewportChange); - scrollBox.viewport.off("resized", handleViewportChange); + scrollBox.viewport.off("resize", handleViewportResize); }; - }, [activateRapidScrollOverscan, clearAddNoteHoverForScroll, files.length, scrollRef, wrapLines]); + }, [ + activateRapidScrollOverscan, + clearAddNoteHoverForScroll, + files.length, + height, + scrollRef, + wrapLines, + ]); const sectionHeaderHeights = useMemo(() => buildInStreamFileHeaderHeights(files), [files]); const reserveAddNoteColumn = Boolean(onStartUserNoteAtHunk); @@ -918,17 +973,72 @@ export function DiffPane({ [estimatedBodyHeights, files, sectionHeaderHeights], ); const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; + const previousScrollEdgeRequestIdRef = useRef(scrollEdgeRequest?.id ?? 0); + const pendingScrollEdgeRequest = + scrollEdgeRequest && scrollEdgeRequest.id !== previousScrollEdgeRequestIdRef.current + ? scrollEdgeRequest + : null; + const scrollEdgeViewportHeight = Math.max( + scrollViewport.height, + scrollRef.current?.viewport.height ?? 0, + ); + const requestedScrollEdgeTop = pendingScrollEdgeRequest + ? clampVerticalScrollTop( + pendingScrollEdgeRequest.edge === "bottom" ? totalContentHeight : 0, + totalContentHeight, + scrollEdgeViewportHeight, + ) + : null; + const renderScrollTop = requestedScrollEdgeTop ?? scrollViewport.top; + + // Edge jumps render their destination rows before moving OpenTUI's native viewport, avoiding a + // frame where viewport culling points at rows that React has not mounted yet. + useLayoutEffect(() => { + if (!pendingScrollEdgeRequest || requestedScrollEdgeTop === null) { + return; + } + const scrollBox = scrollRef.current; + if (!scrollBox) { + return; + } + + supersedePendingSelectionReveal(); + clearPendingFileTopAlign(); + previousScrollEdgeRequestIdRef.current = pendingScrollEdgeRequest.id; + const viewportHeight = scrollBox.viewport.height || scrollEdgeViewportHeight; + const nextTop = clampVerticalScrollTop( + requestedScrollEdgeTop, + totalContentHeight, + viewportHeight, + ); + setScrollViewport({ top: nextTop, height: viewportHeight }); + scrollBox.scrollTo(nextTop); + }, [ + clearPendingFileTopAlign, + pendingScrollEdgeRequest, + requestedScrollEdgeTop, + scrollEdgeViewportHeight, + scrollRef, + supersedePendingSelectionReveal, + totalContentHeight, + ]); const fileSectionIndexById = useMemo( () => buildFileSectionIndexById(fileSectionLayouts), [fileSectionLayouts], ); - const lineCursors = useMemo( + const measuredLineCursors = useMemo( // Nothing reads the stops while the marker is off, and enumerating them costs one object per // rendered row of the whole changeset every time geometry is remeasured. () => (cursorLine === "off" ? EMPTY_LINE_CURSORS : buildLineCursors(files, sectionGeometry)), [cursorLine, files, sectionGeometry], ); + const previousLineCursorsRef = useRef(EMPTY_LINE_CURSORS); + const lineCursors = reuseEquivalentLineCursors( + previousLineCursorsRef.current, + measuredLineCursors, + ); + previousLineCursorsRef.current = lineCursors; /** Locate one measured row in whole-stream rows, addressed by its file and plan anchor. */ const rowBoundsInStream = useCallback( (fileId: string, stableKey: string) => { @@ -952,7 +1062,9 @@ export function DiffPane({ // 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; + const effectiveScrollTop = pendingScrollEdgeRequest + ? renderScrollTop + : (scrollRef.current?.scrollTop ?? scrollViewport.top); const pinnedHeaderFile = useMemo(() => { if (files.length === 0) { return null; @@ -1434,7 +1546,7 @@ export function DiffPane({ adjacentPrefetchFileIds, fileSectionLayouts, rapidScrollOverscanRows, - scrollTop: scrollViewport.top, + scrollTop: renderScrollTop, viewportHeight: scrollViewport.height, selectedFileId, }), @@ -1443,7 +1555,7 @@ export function DiffPane({ fileSectionLayouts, rapidScrollOverscanRows, scrollViewport.height, - scrollViewport.top, + renderScrollTop, selectedFileId, ], ); @@ -1553,6 +1665,17 @@ export function DiffPane({ files.map((file, sectionIndex) => ({ kind: "file", fileId: file.id, sectionIndex })), [files], ); + const initialRenderViewportHeight = estimateInitialRenderViewportHeight( + renderer.height, + screenTop, + height, + ); + // File windowing must not see height 0: that range is only the first file plus one overscan + // neighbor, which leaves a tall first paint blank until the scrollbox later publishes geometry. + const fileWindowViewportHeight = resolveRenderViewportHeight( + scrollViewport.height, + initialRenderViewportHeight, + ); const fileRenderWindow = useMemo( () => windowingEnabled @@ -1560,16 +1683,16 @@ export function DiffPane({ fileSectionLayouts, indexByFileId: fileSectionIndexById, overscanFiles: 1, - scrollTop: scrollViewport.top, + scrollTop: renderScrollTop, selectedFileId, - viewportHeight: scrollViewport.height, + viewportHeight: fileWindowViewportHeight, }) : null, [ fileSectionIndexById, fileSectionLayouts, - scrollViewport.height, - scrollViewport.top, + fileWindowViewportHeight, + renderScrollTop, selectedFileId, windowingEnabled, ], @@ -1607,10 +1730,6 @@ export function DiffPane({ // back the prior object when top/height are numerically unchanged lets mounted sections skip // re-rendering even though the Map itself is rebuilt every snapshot. const previousVisibleBodyBoundsRef = useRef>(new Map()); - const initialRenderViewportHeight = estimateInitialRenderViewportHeight( - renderer.height, - screenTop, - ); const visibleBodyBoundsByFile = useMemo(() => { const previous = previousVisibleBodyBoundsRef.current; const next = new Map(); @@ -1649,9 +1768,9 @@ export function DiffPane({ // Convert the absolute review-stream viewport into file-body-local coordinates. // Example: if the viewport starts at row 2_000 globally and this file body starts at row // 1_940, then the file-local visible top is 60 rows into this file. - let minTop = scrollViewport.top - sectionLayout.bodyTop - overscanTerminalRows; + let minTop = renderScrollTop - sectionLayout.bodyTop - overscanTerminalRows; let maxBottom = - scrollViewport.top + renderViewportHeight - sectionLayout.bodyTop + overscanTerminalRows; + renderScrollTop + renderViewportHeight - sectionLayout.bodyTop + overscanTerminalRows; // A fitting selected hunk must remain fully mounted even during the zero-halo first paint. // Oversized hunks keep ordinary viewport windowing so one selection cannot defeat startup. @@ -1689,7 +1808,7 @@ export function DiffPane({ initialWrappedRenderWindowWarmed, rapidScrollOverscanRows, scrollViewport.height, - scrollViewport.top, + renderScrollTop, initialRenderViewportHeight, sectionGeometry, mountedFileIndices, @@ -1794,31 +1913,6 @@ export function DiffPane({ const selectedFileBodyTop = selectedFileIndex >= 0 ? (fileSectionLayouts[selectedFileIndex]?.bodyTop ?? 0) : 0; - // Track the previous selected anchor to detect actual selection changes. - const prevSelectedAnchorIdRef = useRef(null); - const prevPinnedHeaderFileIdRef = useRef(null); - const pendingSelectionSettleRef = useRef(false); - const pendingSelectionRevealTimeoutsRef = useRef[]>([]); - - /** Clear scheduled selection-reveal retries without changing the resettle policy. */ - const clearPendingSelectionRevealTimers = useCallback(() => { - for (const timeout of pendingSelectionRevealTimeoutsRef.current) { - clearTimeout(timeout); - } - pendingSelectionRevealTimeoutsRef.current = []; - }, []); - - /** Retire selection reveal work once an explicit line alignment becomes authoritative. */ - const supersedePendingSelectionReveal = useCallback(() => { - clearPendingSelectionRevealTimers(); - pendingSelectionSettleRef.current = false; - }, [clearPendingSelectionRevealTimers]); - - /** Clear any pending "selected file to top" follow-up. */ - const clearPendingFileTopAlign = useCallback(() => { - pendingFileTopAlignFileIdRef.current = null; - }, []); - /** * Report whether the align has landed as far as the rest of this pane can observe it. * diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 2b7412505..d9fc650e5 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1220,6 +1220,34 @@ describe("UI components", () => { } }); + test("DiffPane first nowrap paint fills a tall viewport past the overscan neighbor", async () => { + const files = createWindowingFiles(8); + const theme = resolveTheme("github-dark-default", null); + const props = createDiffPaneProps(files, theme, { + diffContentWidth: 88, + separatorWidth: 84, + width: 92, + }); + const setup = await testRender(, { + width: 96, + height: 40, + }); + + try { + await act(async () => { + await setup.renderOnce(); + }); + const frame = setup.captureCharFrame(); + + expect(frame).toContain("window-3.ts"); + expect(frame).toContain("file3Extra = true"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane scrolls a later selected file into view in the windowed path", async () => { const files = createWindowingFiles(6); const theme = resolveTheme("github-dark-default", null); diff --git a/src/ui/lib/fileRenderWindow.test.ts b/src/ui/lib/fileRenderWindow.test.ts index edf18c5dd..b58dee453 100644 --- a/src/ui/lib/fileRenderWindow.test.ts +++ b/src/ui/lib/fileRenderWindow.test.ts @@ -56,6 +56,20 @@ describe("buildFileRenderWindow", () => { expect(plan.bottomSpacerHeight).toBe(0); }); + test("a zero-height viewport at the top only mounts the first file plus overscan", () => { + const layouts = createLayouts(6, 8); + const plan = buildFileRenderWindow({ + fileSectionLayouts: layouts, + overscanFiles: 1, + scrollTop: 0, + viewportHeight: 0, + }); + + expect(plan.mountedFileIndices).toEqual([0, 1]); + expect(plan.visibleStartIndex).toBe(0); + expect(plan.visibleEndIndex).toBe(0); + }); + test("mounts the visible first file and reserves the rest in one bottom spacer", () => { const layouts = createLayouts(4); const plan = buildFileRenderWindow({ diff --git a/src/ui/lib/lineCursors.test.ts b/src/ui/lib/lineCursors.test.ts index f88eda01e..6ddfb3596 100644 --- a/src/ui/lib/lineCursors.test.ts +++ b/src/ui/lib/lineCursors.test.ts @@ -15,6 +15,7 @@ import { findLineCursorAt, findNextLineCursor, firstLineCursorInHunk, + reuseEquivalentLineCursors, resolveLineCursor, type LineCursor, } from "./lineCursors"; @@ -196,6 +197,26 @@ describe("buildLineCursors", () => { }); }); +describe("reuseEquivalentLineCursors", () => { + test("keeps list identity when remeasurement preserves every cursor", () => { + const previous = cursorsFor([createContextWrappedFile("alpha", "alpha.ts")], "stack"); + const next = previous.map((cursor) => ({ ...cursor, target: { ...cursor.target } })); + + expect(reuseEquivalentLineCursors(previous, next)).toBe(previous); + }); + + test("keeps a changed cursor list", () => { + const previous = cursorsFor([createContextWrappedFile("alpha", "alpha.ts")], "stack"); + const next = previous.map((cursor, index) => + index === 0 + ? { ...cursor, target: { ...cursor.target, line: cursor.target.line + 1 } } + : cursor, + ); + + expect(reuseEquivalentLineCursors(previous, next)).toBe(next); + }); +}); + describe("findLineCursorAt", () => { /** Build a file whose inserted line pushes the trailing context onto different side numbers. */ function createShiftedContextFile() { diff --git a/src/ui/lib/lineCursors.ts b/src/ui/lib/lineCursors.ts index 510c576c9..603781495 100644 --- a/src/ui/lib/lineCursors.ts +++ b/src/ui/lib/lineCursors.ts @@ -106,6 +106,27 @@ export function buildLineCursors( }); } +/** Reuse the previous ordered cursor list when remeasurement preserved every navigation target. */ +export function reuseEquivalentLineCursors(previous: LineCursor[], next: LineCursor[]) { + if ( + previous.length === next.length && + next.every((cursor, index) => { + const prior = previous[index]; + return ( + prior?.fileId === cursor.fileId && + prior.hunkIndex === cursor.hunkIndex && + prior.stableKey === cursor.stableKey && + prior.expandedGapKey === cursor.expandedGapKey && + prior.target.side === cursor.target.side && + prior.target.line === cursor.target.line + ); + }) + ) { + return previous; + } + return next; +} + /** Find the first cursor in one hunk, then anywhere in its file. */ function nearestCursorInFile(cursors: LineCursor[], fileId: string, hunkIndex: number) { return ( diff --git a/src/ui/lib/viewportTiming.test.ts b/src/ui/lib/viewportTiming.test.ts new file mode 100644 index 000000000..1c14287ec --- /dev/null +++ b/src/ui/lib/viewportTiming.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, test } from "bun:test"; +import { estimateInitialRenderViewportHeight, resolveRenderViewportHeight } from "./viewportTiming"; + +describe("estimateInitialRenderViewportHeight", () => { + test("subtracts the pane's screen top from the renderer height", () => { + expect(estimateInitialRenderViewportHeight(80, 2)).toBe(78); + }); + + test("never returns an empty window while geometry is still unknown", () => { + expect(estimateInitialRenderViewportHeight(0, 0)).toBe(1); + }); + + test("does not include a bottom pane in the review viewport estimate", () => { + expect(estimateInitialRenderViewportHeight(100, 1, 5)).toBe(5); + }); +}); + +describe("resolveRenderViewportHeight", () => { + test("falls back to the estimate while the scrollbox height is still 0", () => { + expect(resolveRenderViewportHeight(0, 48)).toBe(48); + }); + + test("keeps the measured scrollbox height once it is available", () => { + expect(resolveRenderViewportHeight(36, 48)).toBe(36); + }); +}); diff --git a/src/ui/lib/viewportTiming.ts b/src/ui/lib/viewportTiming.ts index b42433305..83fb2103d 100644 --- a/src/ui/lib/viewportTiming.ts +++ b/src/ui/lib/viewportTiming.ts @@ -1,2 +1,31 @@ /** Delay used to coalesce imperative ScrollBox viewport reads to roughly one frame. */ export const VIEWPORT_READ_COALESCE_MS = 16; + +/** + * Estimate render-only viewport bounds before OpenTUI publishes exact scrollbox geometry. + * Subtracts the review pane's screen-top offset from the renderer height so the first paint + * can window files without waiting for the scrollbox to report its laid-out height. A planned + * pane height excludes any extension pane below the review. + */ +export function estimateInitialRenderViewportHeight( + rendererHeight: number, + screenTop: number, + paneHeight?: number, +) { + const availableRendererHeight = rendererHeight - Math.max(0, screenTop); + return Math.max( + 1, + paneHeight === undefined + ? availableRendererHeight + : Math.min(availableRendererHeight, paneHeight), + ); +} + +/** + * Prefer the measured scrollbox height once it exists; otherwise keep the first-paint estimate. + * Passing a measured 0 into file windowing only mounts the leading file plus overscan, which + * leaves a tall first frame blank until the user scrolls. + */ +export function resolveRenderViewportHeight(measuredHeight: number, estimatedHeight: number) { + return measuredHeight > 0 ? measuredHeight : Math.max(1, estimatedHeight); +} diff --git a/test/pty/harness.ts b/test/pty/harness.ts index 9874dfb4d..60ce1ace9 100644 --- a/test/pty/harness.ts +++ b/test/pty/harness.ts @@ -726,6 +726,16 @@ end ]); } + /** Build many short files so a tall first paint must mount past the first-file overscan neighbor. */ + function createManyShortFileRepoFixture() { + return createGitRepoFixture( + Array.from({ length: 8 }, (_, index) => ({ + path: `short-${index}.ts`, + before: `export const short${index} = ${index};\n`, + after: `export const short${index} = ${index + 10};\n`, + })), + ); + } function createPinnedHeaderRepoFixture() { return createGitRepoFixture([ { @@ -1093,6 +1103,7 @@ end createMultiHunkFilePair, createNarrowHeaderTestRepoFixture, createPagerPatchFixture, + createManyShortFileRepoFixture, createPinnedHeaderRepoFixture, createRapidThemePreviewTestRepoFixture, createScrollableFilePair, diff --git a/test/pty/layout.test.ts b/test/pty/layout.test.ts index a3ac1c3d2..dd005b753 100644 --- a/test/pty/layout.test.ts +++ b/test/pty/layout.test.ts @@ -43,6 +43,35 @@ describe("PTY layout", () => { } }); + test("the first nowrap frame fills a tall viewport past the first-file overscan neighbor", async () => { + // File windowing with a still-unmeasured (0) viewport only mounts the leading file plus one + // overscan neighbor. A tall first paint must use the estimated height so later short files + // appear without any scroll input. + const fixture = harness.createManyShortFileRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "stack", "--no-sidebar"], + cwd: fixture.dir, + cols: 100, + rows: 40, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Agent\s+Help/, { + timeout: 15_000, + }); + + const firstFrame = await harness.waitForSnapshot( + session, + (text) => text.includes("short-3.ts") && text.includes("short3 = 13"), + 8_000, + ); + + expect(firstFrame).toContain("short-3.ts"); + expect(firstFrame).toContain("short3 = 13"); + } finally { + session.close(); + } + }); test("split rows keep the center separator aligned after wide characters", async () => { const fixture = harness.createWideCharacterFilePair(); const session = await harness.launchHunk({