diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 8df246f..04bcdbd 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -1,4 +1,5 @@ import type { ScrollBoxRenderable } from "@opentui/core"; +import { useRenderer } from "@opentui/react"; import { useCallback, useEffect, @@ -18,8 +19,9 @@ import { } from "../../lib/diffSectionGeometry"; import { buildFileSectionLayouts, + buildInStreamFileHeaderHeights, findHeaderOwningFileSection, - getFileSectionHeaderTop, + shouldRenderInStreamFileHeader, } from "../../lib/fileSectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; import type { AppTheme } from "../../themes"; @@ -63,10 +65,12 @@ function findViewportRowAnchor( files: DiffFile[], sectionGeometry: DiffSectionGeometry[], scrollTop: number, + headerHeights: number[], ) { const fileSectionLayouts = buildFileSectionLayouts( files, sectionGeometry.map((metrics) => metrics?.bodyHeight ?? 0), + headerHeights, ); for (let index = 0; index < files.length; index += 1) { @@ -96,10 +100,12 @@ function resolveViewportRowAnchorTop( files: DiffFile[], sectionGeometry: DiffSectionGeometry[], anchor: ViewportRowAnchor, + headerHeights: number[], ) { const fileSectionLayouts = buildFileSectionLayouts( files, sectionGeometry.map((metrics) => metrics?.bodyHeight ?? 0), + headerHeights, ); for (let index = 0; index < files.length; index += 1) { @@ -165,6 +171,7 @@ export function DiffPane({ onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; onSelectFile: (fileId: string) => void; }) { + const renderer = useRenderer(); const [prefetchAnchorKey, setPrefetchAnchorKey] = useState(null); const selectedHighlightKey = selectedFileId ? `${theme.appearance}:${selectedFileId}` : null; @@ -245,11 +252,16 @@ export function DiffPane({ const pendingFileTopAlignFileIdRef = useRef(null); useEffect(() => { + const scrollBox = scrollRef.current; + if (!scrollBox) { + return; + } + const updateViewport = () => { - const nextTop = scrollRef.current?.scrollTop ?? 0; - const nextHeight = scrollRef.current?.viewport.height ?? 0; + const nextTop = scrollBox.scrollTop ?? 0; + const nextHeight = scrollBox.viewport.height ?? 0; - // Detect scroll activity and show scrollbar + // Detect scroll activity and show scrollbar. if (nextTop !== prevScrollTopRef.current) { scrollbarRef.current?.show(); prevScrollTopRef.current = nextTop; @@ -262,10 +274,23 @@ export function DiffPane({ ); }; + const handleViewportChange = () => { + updateViewport(); + }; + updateViewport(); - const interval = setInterval(updateViewport, 50); - return () => clearInterval(interval); - }, [scrollRef]); + scrollBox.verticalScrollBar.on("change", handleViewportChange); + scrollBox.viewport.on("layout-changed", handleViewportChange); + scrollBox.viewport.on("resized", handleViewportChange); + + return () => { + scrollBox.verticalScrollBar.off("change", handleViewportChange); + scrollBox.viewport.off("layout-changed", handleViewportChange); + scrollBox.viewport.off("resized", handleViewportChange); + }; + }, [files.length, scrollRef]); + + const sectionHeaderHeights = useMemo(() => buildInStreamFileHeaderHeights(files), [files]); const baseSectionGeometry = useMemo( () => @@ -288,8 +313,8 @@ export function DiffPane({ [baseSectionGeometry], ); const baseFileSectionLayouts = useMemo( - () => buildFileSectionLayouts(files, baseEstimatedBodyHeights), - [baseEstimatedBodyHeights, files], + () => buildFileSectionLayouts(files, baseEstimatedBodyHeights, sectionHeaderHeights), + [baseEstimatedBodyHeights, files, sectionHeaderHeights], ); const visibleViewportFileIds = useMemo(() => { @@ -363,27 +388,34 @@ export function DiffPane({ [sectionGeometry], ); const fileSectionLayouts = useMemo( - () => buildFileSectionLayouts(files, estimatedBodyHeights), - [estimatedBodyHeights, files], + () => buildFileSectionLayouts(files, estimatedBodyHeights, sectionHeaderHeights), + [estimatedBodyHeights, files, sectionHeaderHeights], ); - // Read the live scroll box position during render so sticky-header ownership flips + const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; + // 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 totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; - - const stickyHeaderFile = useMemo(() => { - if (files.length < 2) { + const pinnedHeaderFile = useMemo(() => { + if (files.length === 0) { return null; } - const owner = findHeaderOwningFileSection(fileSectionLayouts, effectiveScrollTop); - if (!owner || effectiveScrollTop <= owner.headerTop) { - return null; - } + // The current file header always owns the pinned top row. + // Use the previous visible row to decide ownership so the next file's real header can still + // scroll through the stream before the pinned header hands off to it on the following row. + const owner = findHeaderOwningFileSection( + fileSectionLayouts, + Math.max(0, effectiveScrollTop - 1), + ); - return files[owner.sectionIndex] ?? null; + return owner ? (files[owner.sectionIndex] ?? null) : (files[0] ?? null); }, [effectiveScrollTop, fileSectionLayouts, files]); + const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; + + useLayoutEffect(() => { + renderer.intermediateRender(); + }, [renderer, pinnedHeaderFileId]); const visibleWindowedFileIds = useMemo(() => { if (!windowingEnabled) { @@ -473,17 +505,19 @@ export function DiffPane({ // Track the previous selected anchor to detect actual selection changes. const prevSelectedAnchorIdRef = useRef(null); + const prevPinnedHeaderFileIdRef = useRef(null); + const pendingSelectionSettleRef = useRef(false); /** Clear any pending "selected file to top" follow-up. */ const clearPendingFileTopAlign = useCallback(() => { pendingFileTopAlignFileIdRef.current = null; }, []); - /** Scroll one file header to the top using the latest planned section geometry. */ + /** Scroll one file so it immediately owns the viewport top using the latest planned geometry. */ const scrollFileHeaderToTop = useCallback( (fileId: string) => { - const headerTop = getFileSectionHeaderTop(fileSectionLayouts, fileId); - if (headerTop == null) { + const targetSection = fileSectionLayouts.find((layout) => layout.fileId === fileId); + if (!targetSection) { return false; } @@ -492,7 +526,8 @@ export function DiffPane({ return false; } - scrollBox.scrollTo(headerTop); + // The pinned header owns the top row, so align the review stream to the file body. + scrollBox.scrollTo(targetSection.bodyTop); return true; }, [fileSectionLayouts, scrollRef], @@ -502,6 +537,7 @@ export function DiffPane({ const wrapChanged = previousWrapLinesRef.current !== wrapLines; const previousSectionMetrics = previousSectionGeometryRef.current; const previousFiles = previousFilesRef.current; + const previousSectionHeaderHeights = buildInStreamFileHeaderHeights(previousFiles); if (wrapChanged && previousSectionMetrics && previousFiles.length > 0) { const previousScrollTop = @@ -514,9 +550,15 @@ export function DiffPane({ previousFiles, previousSectionMetrics, previousScrollTop, + previousSectionHeaderHeights, ); if (anchor) { - const nextTop = resolveViewportRowAnchorTop(files, sectionGeometry, anchor); + const nextTop = resolveViewportRowAnchorTop( + files, + sectionGeometry, + anchor, + sectionHeaderHeights, + ); const restoreViewportAnchor = () => { scrollRef.current?.scrollTo(nextTop); }; @@ -542,7 +584,15 @@ export function DiffPane({ previousWrapLinesRef.current = wrapLines; previousSectionGeometryRef.current = sectionGeometry; previousFilesRef.current = files; - }, [files, scrollRef, scrollViewport.top, sectionGeometry, wrapLines, wrapToggleScrollTop]); + }, [ + files, + scrollRef, + scrollViewport.top, + sectionGeometry, + sectionHeaderHeights, + wrapLines, + wrapToggleScrollTop, + ]); useLayoutEffect(() => { if (previousSelectedFileTopAlignRequestIdRef.current === selectedFileTopAlignRequestId) { @@ -581,11 +631,13 @@ export function DiffPane({ return; } - const desiredTop = getFileSectionHeaderTop(fileSectionLayouts, pendingFileId); - if (desiredTop == null) { + const targetSection = fileSectionLayouts.find((layout) => layout.fileId === pendingFileId); + if (!targetSection) { return; } + const desiredTop = targetSection.bodyTop; + const currentTop = scrollRef.current?.scrollTop ?? scrollViewport.top; if (Math.abs(currentTop - desiredTop) <= 0.5) { clearPendingFileTopAlign(); @@ -603,24 +655,40 @@ export function DiffPane({ ]); useLayoutEffect(() => { + const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; + if (suppressNextSelectionAutoScrollRef.current) { suppressNextSelectionAutoScrollRef.current = false; // Consume this selection transition so the next render does not re-center the selected hunk. prevSelectedAnchorIdRef.current = selectedAnchorId; + prevPinnedHeaderFileIdRef.current = pinnedHeaderFileId; + pendingSelectionSettleRef.current = false; return; } if (!selectedAnchorId && !selectedEstimatedHunkBounds) { prevSelectedAnchorIdRef.current = null; + prevPinnedHeaderFileIdRef.current = pinnedHeaderFileId; + pendingSelectionSettleRef.current = false; return; } + const shouldTrackPinnedHeaderResettle = + selectedFileIndex > 0 || selectedHunkIndex > 0 || selectedNoteBounds !== null; + // Only auto-scroll when the selection actually changes, not when geometry updates during - // scrolling or when the selected section refines its measured bounds. + // scrolling or when the selected section refines its measured bounds. One exception: after a + // programmatic jump to a later file/hunk, rerun the settle scroll once if the pinned header + // hands off to a different file while the selected content is still settling. const isSelectionChange = prevSelectedAnchorIdRef.current !== selectedAnchorId; + const pinnedHeaderChangedWhileSettling = + shouldTrackPinnedHeaderResettle && + pendingSelectionSettleRef.current && + prevPinnedHeaderFileIdRef.current !== pinnedHeaderFileId; prevSelectedAnchorIdRef.current = selectedAnchorId; + prevPinnedHeaderFileIdRef.current = pinnedHeaderFileId; - if (!isSelectionChange) { + if (!isSelectionChange && !pinnedHeaderChangedWhileSettling) { return; } @@ -687,17 +755,29 @@ export function DiffPane({ // Run after this pane renders the selected section/hunk, then retry briefly while layout // settles across a couple of repaint cycles. scrollSelectionIntoView(); + pendingSelectionSettleRef.current = shouldTrackPinnedHeaderResettle; const retryDelays = [0, 16, 48]; const timeouts = retryDelays.map((delay) => setTimeout(scrollSelectionIntoView, delay)); + const settleReset = shouldTrackPinnedHeaderResettle + ? setTimeout(() => { + pendingSelectionSettleRef.current = false; + }, 120) + : null; return () => { timeouts.forEach((timeout) => clearTimeout(timeout)); + if (settleReset) { + clearTimeout(settleReset); + } }; }, [ scrollRef, scrollViewport.height, selectedAnchorId, selectedEstimatedHunkBounds, + selectedFileIndex, + selectedHunkIndex, selectedNoteBounds, + pinnedHeaderFile?.id, ]); // Configure scroll step size to scroll exactly 1 line per step @@ -722,15 +802,15 @@ export function DiffPane({ > {files.length > 0 ? ( - {/* Keep the current file header visible once its real header has scrolled offscreen. */} - {stickyHeaderFile ? ( + {/* Always pin the current file header in a dedicated top row. */} + {pinnedHeaderFile ? ( onSelectFile(stickyHeaderFile.id)} + onSelect={() => onSelectFile(pinnedHeaderFile.id)} /> ) : null} @@ -763,7 +843,7 @@ export function DiffPane({ visibleViewportFileIds.has(file.id); // Windowing keeps offscreen files cheap: render placeholders with identical - // section geometry so scroll math and sticky-header ownership stay stable. + // section geometry so scroll math and pinned-header ownership stay stable. if (!shouldRenderSection) { return ( 0} theme={theme} onSelect={() => onSelectFile(file.id)} @@ -787,7 +868,6 @@ export function DiffPane({ headerLabelWidth={headerLabelWidth} headerStatsWidth={headerStatsWidth} layout={layout} - selected={file.id === selectedFileId} selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1} shouldLoadHighlight={ file.id === selectedFileId || @@ -798,6 +878,7 @@ export function DiffPane({ file.id === selectedFileId ? handleSelectedHighlightReady : undefined } separatorWidth={separatorWidth} + showHeader={shouldRenderInStreamFileHeader(index)} showSeparator={index > 0} showLineNumbers={showLineNumbers} showHunkHeaders={showHunkHeaders} diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index a3c01bb..b6aceea 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -12,7 +12,6 @@ interface DiffSectionProps { headerLabelWidth: number; headerStatsWidth: number; layout: Exclude; - selected: boolean; selectedHunkIndex: number; shouldLoadHighlight: boolean; onHighlightReady?: () => void; @@ -20,6 +19,7 @@ interface DiffSectionProps { showLineNumbers: boolean; showHunkHeaders: boolean; wrapLines: boolean; + showHeader: boolean; showSeparator: boolean; theme: AppTheme; visibleAgentNotes: VisibleAgentNote[]; @@ -34,7 +34,6 @@ function DiffSectionComponent({ headerLabelWidth, headerStatsWidth, layout, - selected: _selected, selectedHunkIndex, shouldLoadHighlight, onHighlightReady, @@ -42,6 +41,7 @@ function DiffSectionComponent({ showLineNumbers, showHunkHeaders, wrapLines, + showHeader, showSeparator, theme, visibleAgentNotes, @@ -75,13 +75,15 @@ function DiffSectionComponent({ ) : null} - + {showHeader ? ( + + ) : null} { previous.headerLabelWidth === next.headerLabelWidth && previous.headerStatsWidth === next.headerStatsWidth && previous.layout === next.layout && - previous.selected === next.selected && previous.selectedHunkIndex === next.selectedHunkIndex && previous.shouldLoadHighlight === next.shouldLoadHighlight && previous.separatorWidth === next.separatorWidth && previous.showLineNumbers === next.showLineNumbers && previous.showHunkHeaders === next.showHunkHeaders && previous.wrapLines === next.wrapLines && + previous.showHeader === next.showHeader && previous.showSeparator === next.showSeparator && previous.theme === next.theme && previous.visibleAgentNotes === next.visibleAgentNotes && diff --git a/src/ui/components/panes/DiffSectionPlaceholder.tsx b/src/ui/components/panes/DiffSectionPlaceholder.tsx index 14e29c9..0d3e797 100644 --- a/src/ui/components/panes/DiffSectionPlaceholder.tsx +++ b/src/ui/components/panes/DiffSectionPlaceholder.tsx @@ -10,6 +10,7 @@ interface DiffSectionPlaceholderProps { headerLabelWidth: number; headerStatsWidth: number; separatorWidth: number; + showHeader: boolean; showSeparator: boolean; theme: AppTheme; onSelect: () => void; @@ -22,6 +23,7 @@ export function DiffSectionPlaceholder({ headerLabelWidth, headerStatsWidth, separatorWidth, + showHeader, showSeparator, theme, onSelect, @@ -49,13 +51,15 @@ export function DiffSectionPlaceholder({ ) : null} - + {showHeader ? ( + + ) : null} diff --git a/src/ui/lib/fileSectionLayout.ts b/src/ui/lib/fileSectionLayout.ts index ed39f8a..622049b7 100644 --- a/src/ui/lib/fileSectionLayout.ts +++ b/src/ui/lib/fileSectionLayout.ts @@ -11,17 +11,37 @@ export interface FileSectionLayout { sectionBottom: number; } -/** Build absolute section offsets from file order and measured body heights. */ -export function buildFileSectionLayouts(files: DiffFile[], bodyHeights: number[]) { +/** Return the in-stream header height for one review section. */ +export function getInStreamFileHeaderHeight(sectionIndex: number) { + return sectionIndex === 0 ? 0 : 1; +} + +/** Return whether one review section should render its in-stream file header. */ +export function shouldRenderInStreamFileHeader(sectionIndex: number) { + return getInStreamFileHeaderHeight(sectionIndex) > 0; +} + +/** Build the in-stream header heights for the current review stream. */ +export function buildInStreamFileHeaderHeights(files: DiffFile[]) { + return files.map((_, index) => getInStreamFileHeaderHeight(index)); +} + +/** Build absolute section offsets from file order, header heights, and measured body heights. */ +export function buildFileSectionLayouts( + files: DiffFile[], + bodyHeights: number[], + headerHeights?: number[], +) { const layouts: FileSectionLayout[] = []; let cursor = 0; files.forEach((file, index) => { const separatorHeight = index > 0 ? 1 : 0; + const headerHeight = Math.max(0, headerHeights?.[index] ?? getInStreamFileHeaderHeight(index)); const bodyHeight = Math.max(0, bodyHeights[index] ?? 0); const sectionTop = cursor; const headerTop = sectionTop + separatorHeight; - const bodyTop = headerTop + 1; + const bodyTop = headerTop + headerHeight; const sectionBottom = bodyTop + bodyHeight; layouts.push({ @@ -69,13 +89,3 @@ export function findHeaderOwningFileSection( return fileSectionLayouts[winner]!; } - -/** Return the scroll top needed to make one file header own the viewport top. */ -export function getFileSectionHeaderTop(fileSectionLayouts: FileSectionLayout[], fileId: string) { - const targetSection = fileSectionLayouts.find((layout) => layout.fileId === fileId); - if (!targetSection) { - return null; - } - - return targetSection.headerTop; -} diff --git a/test/app-interactions.test.tsx b/test/app-interactions.test.tsx index 3223f49..5fe5141 100644 --- a/test/app-interactions.test.tsx +++ b/test/app-interactions.test.tsx @@ -359,6 +359,46 @@ function createTwoFileHunkBootstrap(): AppBootstrap { }; } +function createCollapsedTopBootstrap(): AppBootstrap { + const beforeLines = Array.from( + { length: 400 }, + (_, index) => `export const line${String(index + 1).padStart(3, "0")} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + afterLines[365] = "export const line366 = 9999;"; + + return { + input: { + kind: "git", + staged: false, + options: { + mode: "split", + }, + }, + changeset: { + id: "changeset:collapsed-top", + sourceLabel: "repo", + title: "repo working tree", + files: [ + createDiffFile( + "late", + "src/ui/components/panes/DiffPane.tsx", + lines(...beforeLines), + lines(...afterLines), + ), + createDiffFile( + "second", + "other.ts", + lines("export const other = 1;"), + lines("export const other = 2;"), + ), + ], + }, + initialMode: "split", + initialTheme: "midnight", + }; +} + async function flush(setup: Awaited>) { await act(async () => { await setup.renderOnce(); @@ -849,6 +889,98 @@ describe("App interactions", () => { } }); + test("the first down-arrow step still advances content under the always-pinned file header above a collapsed gap", async () => { + const setup = await testRender(, { + width: 220, + height: 10, + }); + + try { + await flush(setup); + await act(async () => { + await Bun.sleep(80); + await setup.renderOnce(); + }); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("DiffPane.tsx"); + expect(frame).toContain("··· 362 unchanged lines ···"); + expect(frame).not.toContain("366 - export const line366 = 366;"); + + await act(async () => { + await setup.mockInput.pressArrow("down"); + }); + await flush(setup); + await act(async () => { + await Bun.sleep(80); + await setup.renderOnce(); + }); + + frame = await waitForFrame(setup, (nextFrame) => + nextFrame.includes("366 - export const line366 = 366;"), + ); + expect(frame).toContain("366 - export const line366 = 366;"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("one-line down then up at the top restores the collapsed-gap view beneath the pinned file header", async () => { + const setup = await testRender(, { + width: 220, + height: 10, + }); + + try { + await flush(setup); + await act(async () => { + await Bun.sleep(80); + await setup.renderOnce(); + }); + + const initialFrame = setup.captureCharFrame(); + const initialHeaderCount = (initialFrame.match(/DiffPane\.tsx/g) ?? []).length; + + await act(async () => { + await setup.mockInput.pressArrow("down"); + }); + await flush(setup); + await act(async () => { + await Bun.sleep(80); + await setup.renderOnce(); + }); + + let frame = await waitForFrame(setup, (nextFrame) => + nextFrame.includes("366 - export const line366 = 366;"), + ); + + await act(async () => { + await setup.mockInput.pressArrow("up"); + }); + await flush(setup); + await act(async () => { + await Bun.sleep(80); + await setup.renderOnce(); + }); + + frame = await waitForFrame( + setup, + (nextFrame) => + nextFrame.includes("··· 362 unchanged lines ···") && + (nextFrame.match(/DiffPane\.tsx/g) ?? []).length === initialHeaderCount, + ); + expect(frame).toContain("··· 362 unchanged lines ···"); + expect(frame).not.toContain("366 - export const line366 = 366;"); + expect((frame.match(/DiffPane\.tsx/g) ?? []).length).toBe(initialHeaderCount); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("pager mode arrow keys also scroll line by line", async () => { const setup = await testRender(, { width: 220, @@ -1410,7 +1542,7 @@ describe("App interactions", () => { } }); - test("hunk navigation updates the pinned file header when jumping across files", async () => { + test("hunk navigation makes the destination file own the top of the review pane", async () => { const setup = await testRender(, { width: 220, height: 10, diff --git a/test/integration/tuistory-hunk.integration.ts b/test/integration/tuistory-hunk.integration.ts index d1a49fa..00b59e0 100644 --- a/test/integration/tuistory-hunk.integration.ts +++ b/test/integration/tuistory-hunk.integration.ts @@ -117,7 +117,7 @@ describe("Hunk integration via tuistory", () => { ); expect(secondHunk).toContain("line60 = 6000"); - expect(secondHunk).toContain("line61 = 6100"); + expect(secondHunk).toContain("@@ -57,12 +57,12 @@"); expect(secondHunk).not.toContain("line1 = 100"); } finally { session.close(); @@ -234,6 +234,73 @@ describe("Hunk integration via tuistory", () => { } }); + test("mouse wheel scrolling preserves the divider and header handoff in a real PTY", async () => { + const fixture = harness.createPinnedHeaderRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 10, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).toContain("first.ts"); + expect(initial).toContain("second.ts"); + + await session.scrollDown(17); + const boundary = await harness.waitForSnapshot( + session, + (text) => + harness.countMatches(text, /first\.ts/g) === 2 && + harness.countMatches(text, /second\.ts/g) === 2 && + text.includes("@@ -1,16 +1,16 @@") && + text.includes("line17 = 117"), + 5_000, + ); + + expect(boundary).toContain("first.ts"); + expect(boundary).toContain("second.ts"); + expect(boundary).toContain("@@ -1,16 +1,16 @@"); + expect(boundary).toContain("line17 = 117"); + + await session.scrollDown(1); + const nextHeader = await harness.waitForSnapshot( + session, + (text) => + harness.countMatches(text, /first\.ts/g) === 2 && + harness.countMatches(text, /second\.ts/g) === 2 && + text.includes("line18 = 118"), + 5_000, + ); + + expect(nextHeader).toContain("first.ts"); + expect(nextHeader).toContain("second.ts"); + expect(nextHeader).toContain("line18 = 118"); + + await session.scrollDown(1); + const handedOff = await harness.waitForSnapshot( + session, + (text) => + harness.countMatches(text, /first\.ts/g) === 1 && + harness.countMatches(text, /second\.ts/g) === 2 && + text.includes("line17 = 117") && + !text.includes("@@ -1,16 +1,16 @@"), + 5_000, + ); + + expect(harness.countMatches(handedOff, /first\.ts/g)).toBe(1); + expect(harness.countMatches(handedOff, /second\.ts/g)).toBe(2); + expect(handedOff).toContain("line17 = 117"); + expect(handedOff).not.toContain("@@ -1,16 +1,16 @@"); + } finally { + session.close(); + } + }); + test("explicit split mode stays split after a live resize", async () => { const fixture = harness.createTwoFileRepoFixture(); const session = await harness.launchHunk({ @@ -417,12 +484,14 @@ describe("Hunk integration via tuistory", () => { await session.scrollDown(12); const scrolled = await harness.waitForSnapshot( session, - (text) => text.includes("line11 = 111") && !text.includes("line01 = 101"), + (text) => + !text.includes("line01 = 101") && + (text.includes("line11 = 111") || text.includes("line12 = 112")), 5_000, ); - expect(scrolled).toContain("line11 = 111"); expect(scrolled).not.toContain("line01 = 101"); + expect(scrolled.includes("line11 = 111") || scrolled.includes("line12 = 112")).toBe(true); await session.scrollUp(12); const restored = await harness.waitForSnapshot( @@ -436,4 +505,74 @@ describe("Hunk integration via tuistory", () => { session.close(); } }); + + test("the first mouse-wheel step still advances content under the always-pinned file header above a collapsed gap", async () => { + const fixture = harness.createCollapsedTopRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 10, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).toContain("aaa-collapsed.ts"); + expect(initial).toContain("··· 362 unchanged lines ···"); + expect(initial).not.toContain("366 - export const line366 = 366;"); + + await session.scrollDown(1); + const advanced = await harness.waitForSnapshot( + session, + (text) => text.includes("366 - export const line366 = 366;"), + 5_000, + ); + + expect(advanced).toContain("366 - export const line366 = 366;"); + } finally { + session.close(); + } + }); + + test("one mouse-wheel step down then up restores the collapsed-gap view beneath the pinned file header", async () => { + const fixture = harness.createCollapsedTopRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 10, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + const initialHeaderCount = harness.countMatches(initial, /aaa-collapsed\.ts/g); + + await session.scrollDown(1); + await harness.waitForSnapshot( + session, + (text) => text.includes("366 - export const line366 = 366;"), + 5_000, + ); + + await session.scrollUp(1); + const restored = await harness.waitForSnapshot( + session, + (text) => + text.includes("··· 362 unchanged lines ···") && + harness.countMatches(text, /aaa-collapsed\.ts/g) === initialHeaderCount, + 5_000, + ); + + expect(restored).toContain("··· 362 unchanged lines ···"); + expect(restored).not.toContain("366 - export const line366 = 366;"); + expect(harness.countMatches(restored, /aaa-collapsed\.ts/g)).toBe(initialHeaderCount); + } finally { + session.close(); + } + }); }); diff --git a/test/integration/tuistoryHarness.ts b/test/integration/tuistoryHarness.ts index 7398e78..6495c66 100644 --- a/test/integration/tuistoryHarness.ts +++ b/test/integration/tuistoryHarness.ts @@ -247,6 +247,30 @@ export function createTuistoryHarness() { ]); } + function createCollapsedTopRepoFixture() { + const longBefore = + Array.from( + { length: 400 }, + (_, index) => `export const line${String(index + 1).padStart(3, "0")} = ${index + 1};`, + ).join("\n") + "\n"; + const longAfterLines = longBefore.trimEnd().split("\n"); + longAfterLines[365] = "export const line366 = 9999;"; + const longAfter = `${longAfterLines.join("\n")}\n`; + + return createGitRepoFixture([ + { + path: "aaa-collapsed.ts", + before: longBefore, + after: longAfter, + }, + { + path: "zzz-other.ts", + before: "export const other = 1;\n", + after: "export const other = 2;\n", + }, + ]); + } + function createSidebarJumpRepoFixture() { return createGitRepoFixture([ { @@ -362,6 +386,7 @@ export function createTuistoryHarness() { cleanup, countMatches, createAgentFilePair, + createCollapsedTopRepoFixture, createLongWrapFilePair, createMultiHunkFilePair, createPagerPatchFixture, diff --git a/test/tty-render-smoke.test.ts b/test/tty-render-smoke.test.ts index 71faf6a..d91e53e 100644 --- a/test/tty-render-smoke.test.ts +++ b/test/tty-render-smoke.test.ts @@ -337,7 +337,7 @@ describe("TTY render smoke", () => { }); expect(output).toContain("before_23"); - expect(output).toContain("after_06"); + expect(output).toContain("after_05"); }); ttyTest("general pager mode opens Hunk pager UI for diff-like stdin", async () => { diff --git a/test/ui-components.test.tsx b/test/ui-components.test.tsx index d6eb1cb..c06fb0b 100644 --- a/test/ui-components.test.tsx +++ b/test/ui-components.test.tsx @@ -177,6 +177,22 @@ function createTallDiffFile(id: string, path: string, count: number) { return createDiffFile(id, path, before, after); } +function createCollapsedTopDiffFile( + id: string, + path: string, + totalLines: number, + changedLine: number, +) { + const beforeLines = Array.from( + { length: totalLines }, + (_, index) => `export const line${String(index + 1).padStart(3, "0")} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + afterLines[changedLine - 1] = `export const line${changedLine} = 9999;`; + + return createDiffFile(id, path, lines(...beforeLines), lines(...afterLines)); +} + function createDiffPaneProps( files: DiffFile[], theme = resolveTheme("midnight", null), @@ -452,7 +468,11 @@ describe("UI components", () => { try { await settleDiffPane(setup); - const frame = setup.captureCharFrame(); + const frame = await waitForFrame( + setup, + (nextFrame) => nextFrame.includes("window-6.ts") && nextFrame.includes("file6Extra = true"), + 20, + ); expect(frame).toContain("window-6.ts"); expect(frame).toContain("export const file6Extra = true;"); @@ -506,7 +526,7 @@ describe("UI components", () => { } }); - test("DiffPane keeps the previous file header pinned until the next header reaches the top", async () => { + test("DiffPane keeps the sticky-header lane stable through the divider and next-header handoff", async () => { const theme = resolveTheme("midnight", null); const firstFile = createTallDiffFile("first", "first.ts", 18); const secondFile = createTallDiffFile("second", "second.ts", 18); @@ -534,8 +554,8 @@ describe("UI components", () => { true, false, ).bodyHeight; - const secondHeaderTop = firstBodyHeight + 2; - const separatorTop = secondHeaderTop - 1; + const secondHeaderTop = firstBodyHeight + 1; + const separatorTop = firstBodyHeight; const settleStickyScroll = async () => { await act(async () => { for (let iteration = 0; iteration < 6; iteration += 1) { @@ -558,26 +578,155 @@ describe("UI components", () => { frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("first.ts")); expect(frame).toContain("first.ts"); + const stickyViewportHeight = scrollRef.current?.viewport.height ?? 0; + expect(stickyViewportHeight).toBeGreaterThan(0); await act(async () => { scrollRef.current?.scrollTo(separatorTop); }); await settleStickyScroll(); - frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("first.ts")); + frame = await waitForFrame( + setup, + (nextFrame) => nextFrame.includes("first.ts") && nextFrame.includes("────"), + ); expect(frame).toContain("first.ts"); + expect(frame).toContain("────"); + expect(scrollRef.current?.viewport.height ?? 0).toBe(stickyViewportHeight); await act(async () => { scrollRef.current?.scrollTo(secondHeaderTop); }); await settleStickyScroll(); + frame = await waitForFrame( + setup, + (nextFrame) => nextFrame.includes("first.ts") && nextFrame.includes("second.ts"), + ); + expect(frame).toContain("first.ts"); + expect(frame).toContain("second.ts"); + expect(scrollRef.current?.viewport.height ?? 0).toBe(stickyViewportHeight); + + await act(async () => { + scrollRef.current?.scrollTo(secondHeaderTop + 1); + }); + await settleStickyScroll(); + frame = await waitForFrame( setup, (nextFrame) => nextFrame.includes("second.ts") && !nextFrame.includes("first.ts"), ); expect(frame).not.toContain("first.ts"); expect(frame).toContain("second.ts"); + expect(frame).toContain("@@ -1,18 +1,18 @@"); + expect(scrollRef.current?.viewport.height ?? 0).toBe(stickyViewportHeight); + + await act(async () => { + scrollRef.current?.scrollTo(secondHeaderTop + 2); + }); + await settleStickyScroll(); + + frame = await waitForFrame( + setup, + (nextFrame) => nextFrame.includes("second.ts") && !nextFrame.includes("@@ -1,18 +1,18 @@"), + ); + expect(frame).toContain("second.ts"); + expect(frame).not.toContain("@@ -1,18 +1,18 @@"); + expect(scrollRef.current?.viewport.height ?? 0).toBe(stickyViewportHeight); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("DiffPane advances the review stream under the always-pinned file header above a collapsed gap", async () => { + const theme = resolveTheme("midnight", null); + const firstFile = createCollapsedTopDiffFile("late", "late.ts", 400, 366); + const secondFile = createTallDiffFile("second", "second.ts", 4); + const scrollRef = createRef(); + const props = createDiffPaneProps([firstFile, secondFile], theme, { + diffContentWidth: 88, + headerLabelWidth: 48, + headerStatsWidth: 16, + scrollRef, + separatorWidth: 84, + width: 92, + }); + const setup = await testRender(, { + width: 96, + height: 9, + }); + + try { + await settleDiffPane(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("late.ts"); + expect(frame).toContain("··· 362 unchanged lines ···"); + expect(frame).not.toContain("366 - export const line366 = 366;"); + + await act(async () => { + scrollRef.current?.scrollTo(1); + }); + await settleDiffPane(setup); + + frame = await waitForFrame(setup, (nextFrame) => + nextFrame.includes("366 - export const line366 = 366;"), + ); + expect(frame).toContain("366 - export const line366 = 366;"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("DiffPane returns cleanly to the collapsed-gap view after scrolling back up under the pinned file header", async () => { + const theme = resolveTheme("midnight", null); + const firstFile = createCollapsedTopDiffFile("late", "late.ts", 400, 366); + const secondFile = createTallDiffFile("second", "second.ts", 4); + const scrollRef = createRef(); + const props = createDiffPaneProps([firstFile, secondFile], theme, { + diffContentWidth: 88, + headerLabelWidth: 48, + headerStatsWidth: 16, + scrollRef, + separatorWidth: 84, + width: 92, + }); + const setup = await testRender(, { + width: 96, + height: 9, + }); + + try { + await settleDiffPane(setup); + + await act(async () => { + scrollRef.current?.scrollTo(1); + }); + await settleDiffPane(setup); + + let frame = await waitForFrame(setup, (nextFrame) => + nextFrame.includes("366 - export const line366 = 366;"), + ); + expect((frame.match(/late\.ts/g) ?? []).length).toBe(1); + + await act(async () => { + scrollRef.current?.scrollTo(0); + }); + await settleDiffPane(setup); + + frame = await waitForFrame( + setup, + (nextFrame) => + nextFrame.includes("··· 362 unchanged lines ···") && + (nextFrame.match(/late\.ts/g) ?? []).length === 1, + ); + expect(frame).toContain("··· 362 unchanged lines ···"); + expect(frame).not.toContain("366 - export const line366 = 366;"); + expect((frame.match(/late\.ts/g) ?? []).length).toBe(1); } finally { await act(async () => { setup.renderer.destroy(); @@ -614,7 +763,6 @@ describe("UI components", () => { expect(frame).toContain("14 + export const line14 = 1400;"); expect(frame).toContain("16 - export const line16 = 16;"); expect(frame).toContain("16 + export const line16 = 1600;"); - expect(frame).toContain("export const line19 = 19;"); expect(frame).not.toContain("2 - export const line2 = 2;"); expect(frame).not.toContain("2 + export const line2 = 200;"); } finally { @@ -1216,6 +1364,7 @@ describe("UI components", () => { headerLabelWidth={40} headerStatsWidth={16} separatorWidth={68} + showHeader={true} showSeparator={true} theme={theme} onSelect={() => {}}