diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 36dad82..a43ba37 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -121,18 +121,26 @@ export function App({ const [resizeStartWidth, setResizeStartWidth] = useState(null); const [selectedFileId, setSelectedFileId] = useState(bootstrap.changeset.files[0]?.id ?? ""); const [selectedHunkIndex, setSelectedHunkIndex] = useState(0); + const [selectedFileTopAlignRequestId, setSelectedFileTopAlignRequestId] = useState(0); const [scrollToNote, setScrollToNote] = useState(false); const deferredFilter = useDeferredValue(filter); const pagerMode = Boolean(bootstrap.input.options.pager); const activeTheme = resolveTheme(themeId, renderer.themeMode); - const jumpToFile = useCallback((fileId: string, nextHunkIndex = 0) => { - filesScrollRef.current?.scrollChildIntoView(fileRowId(fileId)); - setSelectedFileId(fileId); - setSelectedHunkIndex(nextHunkIndex); - setScrollToNote(false); - }, []); + const jumpToFile = useCallback( + (fileId: string, nextHunkIndex = 0, options?: { alignFileHeaderTop?: boolean }) => { + filesScrollRef.current?.scrollChildIntoView(fileRowId(fileId)); + setSelectedFileId(fileId); + setSelectedHunkIndex(nextHunkIndex); + setScrollToNote(false); + + if (options?.alignFileHeaderTop) { + setSelectedFileTopAlignRequestId((current) => current + 1); + } + }, + [], + ); const jumpToAnnotatedHunk = useCallback((fileId: string, nextHunkIndex = 0) => { filesScrollRef.current?.scrollChildIntoView(fileRowId(fileId)); @@ -948,7 +956,7 @@ export function App({ width={clampedFilesPaneWidth} onSelectFile={(fileId) => { setFocusArea("files"); - jumpToFile(fileId); + jumpToFile(fileId, 0, { alignFileHeaderTop: true }); }} /> @@ -982,6 +990,7 @@ export function App({ showHunkHeaders={showHunkHeaders} wrapLines={wrapLines} wrapToggleScrollTop={wrapToggleScrollTopRef.current} + selectedFileTopAlignRequestId={selectedFileTopAlignRequestId} theme={activeTheme} width={diffPaneWidth} onOpenAgentNotesAtHunk={openAgentNotesAtHunk} diff --git a/src/ui/components/panes/DiffFileHeaderRow.tsx b/src/ui/components/panes/DiffFileHeaderRow.tsx new file mode 100644 index 0000000..46a6d77 --- /dev/null +++ b/src/ui/components/panes/DiffFileHeaderRow.tsx @@ -0,0 +1,61 @@ +import type { DiffFile } from "../../../core/types"; +import { fileLabelParts } from "../../lib/files"; +import { fitText } from "../../lib/text"; +import type { AppTheme } from "../../themes"; + +interface DiffFileHeaderRowProps { + file: DiffFile; + headerLabelWidth: number; + headerStatsWidth: number; + theme: AppTheme; + onSelect?: () => void; +} + +/** Render one file header row in the review stream or sticky overlay. */ +export function DiffFileHeaderRow({ + file, + headerLabelWidth, + headerStatsWidth, + theme, + onSelect, +}: DiffFileHeaderRowProps) { + const additionsText = `+${file.stats.additions}`; + const deletionsText = `-${file.stats.deletions}`; + const { filename, stateLabel } = fileLabelParts(file); + + return ( + + {/* Clicking the file header jumps the main stream selection without collapsing to a single-file view. */} + + + {fitText(filename, Math.max(1, headerLabelWidth - (stateLabel?.length ?? 0)))} + + {stateLabel && {stateLabel}} + + + {additionsText} + + {deletionsText} + + + ); +} diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 03b83d8..ba6fa59 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -16,9 +16,15 @@ import { type DiffSectionMetrics, type DiffSectionRowMetric, } from "../../lib/sectionHeights"; +import { + buildSectionLayoutMetrics, + findHeaderOwningSection, + resolveFileHeaderTop, +} from "../../lib/sectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; import type { AppTheme } from "../../themes"; import { DiffSection } from "./DiffSection"; +import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; @@ -58,15 +64,14 @@ function findViewportRowAnchor( sectionMetrics: DiffSectionMetrics[], scrollTop: number, ) { - let offsetY = 0; + const sectionLayoutMetrics = buildSectionLayoutMetrics( + files, + sectionMetrics.map((metrics) => metrics?.bodyHeight ?? 0), + ); for (let index = 0; index < files.length; index += 1) { - if (index > 0) { - offsetY += 1; - } - - offsetY += 1; - const bodyTop = offsetY; + const layoutMetric = sectionLayoutMetrics[index]; + const bodyTop = layoutMetric?.bodyTop ?? 0; const metrics = sectionMetrics[index]; const bodyHeight = metrics?.bodyHeight ?? 0; const relativeTop = scrollTop - bodyTop; @@ -81,8 +86,6 @@ function findViewportRowAnchor( } satisfies ViewportRowAnchor; } } - - offsetY = bodyTop + bodyHeight; } return null; @@ -94,15 +97,14 @@ function resolveViewportRowAnchorTop( sectionMetrics: DiffSectionMetrics[], anchor: ViewportRowAnchor, ) { - let offsetY = 0; + const sectionLayoutMetrics = buildSectionLayoutMetrics( + files, + sectionMetrics.map((metrics) => metrics?.bodyHeight ?? 0), + ); for (let index = 0; index < files.length; index += 1) { - if (index > 0) { - offsetY += 1; - } - - offsetY += 1; - const bodyTop = offsetY; + const layoutMetric = sectionLayoutMetrics[index]; + const bodyTop = layoutMetric?.bodyTop ?? 0; const file = files[index]; const metrics = sectionMetrics[index]; if (file?.id === anchor.fileId && metrics) { @@ -112,8 +114,6 @@ function resolveViewportRowAnchorTop( } return bodyTop; } - - offsetY = bodyTop + (metrics?.bodyHeight ?? 0); } return 0; @@ -137,6 +137,7 @@ export function DiffPane({ showHunkHeaders, wrapLines, wrapToggleScrollTop, + selectedFileTopAlignRequestId = 0, theme, width, onOpenAgentNotesAtHunk, @@ -158,6 +159,7 @@ export function DiffPane({ showHunkHeaders: boolean; wrapLines: boolean; wrapToggleScrollTop: number | null; + selectedFileTopAlignRequestId?: number; theme: AppTheme; width: number; onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; @@ -238,7 +240,9 @@ export function DiffPane({ const previousSectionMetricsRef = useRef(null); const previousFilesRef = useRef(files); const previousWrapLinesRef = useRef(wrapLines); + const previousSelectedFileTopAlignRequestIdRef = useRef(selectedFileTopAlignRequestId); const suppressNextSelectionAutoScrollRef = useRef(false); + const pendingFileTopAlignFileIdRef = useRef(null); useEffect(() => { const updateViewport = () => { @@ -283,28 +287,21 @@ export function DiffPane({ () => baseSectionMetrics.map((metrics) => metrics.bodyHeight), [baseSectionMetrics], ); + const baseSectionLayoutMetrics = useMemo( + () => buildSectionLayoutMetrics(files, baseEstimatedBodyHeights), + [baseEstimatedBodyHeights, files], + ); const visibleViewportFileIds = useMemo(() => { const overscanRows = 8; const minVisibleY = Math.max(0, scrollViewport.top - overscanRows); const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows; - let offsetY = 0; - const next = new Set(); - - files.forEach((file, index) => { - const sectionHeight = (index > 0 ? 1 : 0) + 1 + (baseEstimatedBodyHeights[index] ?? 0); - const sectionStart = offsetY; - const sectionEnd = sectionStart + sectionHeight; - - if (sectionEnd >= minVisibleY && sectionStart <= maxVisibleY) { - next.add(file.id); - } - - offsetY = sectionEnd; - }); - - return next; - }, [baseEstimatedBodyHeights, files, scrollViewport.height, scrollViewport.top]); + return new Set( + baseSectionLayoutMetrics + .filter((metric) => metric.sectionBottom >= minVisibleY && metric.sectionTop <= maxVisibleY) + .map((metric) => metric.fileId), + ); + }, [baseSectionLayoutMetrics, scrollViewport.height, scrollViewport.top]); const visibleAgentNotesByFile = useMemo(() => { const next = new Map(); @@ -365,22 +362,29 @@ export function DiffPane({ () => sectionMetrics.map((metrics) => metrics.bodyHeight), [sectionMetrics], ); + const sectionLayoutMetrics = useMemo( + () => buildSectionLayoutMetrics(files, estimatedBodyHeights), + [estimatedBodyHeights, files], + ); + // Read the live scroll box position during render so sticky-header ownership flips + // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. + const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; - // Calculate total content height including separators and headers - const totalContentHeight = useMemo(() => { - let total = 0; - for (let index = 0; index < files.length; index += 1) { - // Separator between files (except first) - if (index > 0) { - total += 1; - } - // File header - total += 1; - // File body - total += estimatedBodyHeights[index] ?? 0; + const totalContentHeight = + sectionLayoutMetrics[sectionLayoutMetrics.length - 1]?.sectionBottom ?? 0; + + const stickyHeaderFile = useMemo(() => { + if (files.length < 2) { + return null; } - return total; - }, [files.length, estimatedBodyHeights]); + + const owner = findHeaderOwningSection(sectionLayoutMetrics, effectiveScrollTop); + if (!owner || effectiveScrollTop <= owner.headerTop) { + return null; + } + + return files[owner.sectionIndex] ?? null; + }, [effectiveScrollTop, files, sectionLayoutMetrics]); const visibleWindowedFileIds = useMemo(() => { if (!windowingEnabled) { @@ -414,19 +418,11 @@ export function DiffPane({ return null; } - // Convert the selected hunk's file-local bounds into absolute scrollbox coordinates by adding - // the accumulated section chrome and earlier file heights. - let sectionTop = 0; - for (let index = 0; index < selectedFileIndex; index += 1) { - sectionTop += (index > 0 ? 1 : 0) + 1 + (estimatedBodyHeights[index] ?? 0); - } - - if (selectedFileIndex > 0) { - sectionTop += 1; + const selectedSectionLayout = sectionLayoutMetrics[selectedFileIndex]; + if (!selectedSectionLayout) { + return null; } - sectionTop += 1; - const clampedHunkIndex = Math.max( 0, Math.min(selectedHunkIndex, selectedFile.metadata.hunks.length - 1), @@ -437,13 +433,13 @@ export function DiffPane({ } return { - top: sectionTop + hunkBounds.top, + top: selectedSectionLayout.bodyTop + hunkBounds.top, height: hunkBounds.height, startRowId: hunkBounds.startRowId, endRowId: hunkBounds.endRowId, - sectionTop, + sectionTop: selectedSectionLayout.sectionTop, }; - }, [estimatedBodyHeights, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]); + }, [sectionLayoutMetrics, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]); /** Absolute scroll offset and height of the first inline note in the selected hunk, if any. */ const selectedNoteBounds = useMemo(() => { @@ -479,6 +475,30 @@ export function DiffPane({ // Track the previous selected anchor to detect actual selection changes. const prevSelectedAnchorIdRef = useRef(null); + /** 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. */ + const scrollFileHeaderToTop = useCallback( + (fileId: string) => { + const headerTop = resolveFileHeaderTop(sectionLayoutMetrics, fileId); + if (headerTop == null) { + return false; + } + + const scrollBox = scrollRef.current; + if (!scrollBox) { + return false; + } + + scrollBox.scrollTo(headerTop); + return true; + }, + [scrollRef, sectionLayoutMetrics], + ); + useLayoutEffect(() => { const wrapChanged = previousWrapLinesRef.current !== wrapLines; const previousSectionMetrics = previousSectionMetricsRef.current; @@ -525,9 +545,53 @@ export function DiffPane({ previousFilesRef.current = files; }, [files, scrollRef, scrollViewport.top, sectionMetrics, wrapLines, wrapToggleScrollTop]); + useLayoutEffect(() => { + if (previousSelectedFileTopAlignRequestIdRef.current === selectedFileTopAlignRequestId) { + return; + } + + previousSelectedFileTopAlignRequestIdRef.current = selectedFileTopAlignRequestId; + clearPendingFileTopAlign(); + + if (!selectedFileId || selectedFileIndex < 0) { + return; + } + + // Sidebar navigation should make the selected file immediately own the viewport top. + suppressNextSelectionAutoScrollRef.current = true; + pendingFileTopAlignFileIdRef.current = selectedFileId; + scrollFileHeaderToTop(selectedFileId); + }, [ + clearPendingFileTopAlign, + scrollFileHeaderToTop, + selectedFileTopAlignRequestId, + selectedFileId, + selectedFileIndex, + ]); + + useLayoutEffect(() => { + const pendingFileId = pendingFileTopAlignFileIdRef.current; + if (!pendingFileId) { + return; + } + + // Stop retrying if the sidebar selection points at a file that disappeared mid-settle. + const fileStillPresent = files.some((file) => file.id === pendingFileId); + if (!fileStillPresent) { + clearPendingFileTopAlign(); + return; + } + + if (scrollFileHeaderToTop(pendingFileId)) { + clearPendingFileTopAlign(); + } + }, [clearPendingFileTopAlign, files, scrollFileHeaderToTop, sectionLayoutMetrics]); + useLayoutEffect(() => { if (suppressNextSelectionAutoScrollRef.current) { suppressNextSelectionAutoScrollRef.current = false; + // Consume this selection transition so the next render does not re-center the selected hunk. + prevSelectedAnchorIdRef.current = selectedAnchorId; return; } @@ -642,89 +706,109 @@ export function DiffPane({ }} > {files.length > 0 ? ( - - - - {files.map((file, index) => { - const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true; - const shouldPrefetchVisibleHighlight = - Boolean(selectedHighlightKey) && - prefetchAnchorKey === selectedHighlightKey && - visibleViewportFileIds.has(file.id); - - return shouldRenderSection ? ( - 0} - showLineNumbers={showLineNumbers} - showHunkHeaders={showHunkHeaders} - wrapLines={wrapLines} - theme={theme} - viewWidth={diffContentWidth} - visibleAgentNotes={ - visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES - } - onOpenAgentNotesAtHunk={(hunkIndex) => - onOpenAgentNotesAtHunk(file.id, hunkIndex) - } - onSelect={() => onSelectFile(file.id)} - /> - ) : ( - 0} - theme={theme} - onSelect={() => onSelectFile(file.id)} - /> - ); - })} + + {/* Keep the current file header visible once its real header has scrolled offscreen. */} + {stickyHeaderFile ? ( + + onSelectFile(stickyHeaderFile.id)} + /> - - + ) : null} + + + + {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 sticky-header ownership stay stable. + if (!shouldRenderSection) { + return ( + 0} + theme={theme} + onSelect={() => onSelectFile(file.id)} + /> + ); + } + + return ( + 0} + showLineNumbers={showLineNumbers} + showHunkHeaders={showHunkHeaders} + wrapLines={wrapLines} + theme={theme} + viewWidth={diffContentWidth} + visibleAgentNotes={ + visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES + } + onOpenAgentNotesAtHunk={(hunkIndex) => + onOpenAgentNotesAtHunk(file.id, hunkIndex) + } + onSelect={() => onSelectFile(file.id)} + /> + ); + })} + + + + ) : ( diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index e2a4625..a3c01bb 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -3,9 +3,9 @@ import type { DiffFile, LayoutMode } from "../../../core/types"; import { PierreDiffView } from "../../diff/PierreDiffView"; import { getAnnotatedHunkIndices, type VisibleAgentNote } from "../../lib/agentAnnotations"; import { diffSectionId } from "../../lib/ids"; -import { fileLabelParts } from "../../lib/files"; import { fitText } from "../../lib/text"; import type { AppTheme } from "../../themes"; +import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; interface DiffSectionProps { file: DiffFile; @@ -49,10 +49,7 @@ function DiffSectionComponent({ onOpenAgentNotesAtHunk, onSelect, }: DiffSectionProps) { - const additionsText = `+${file.stats.additions}`; - const deletionsText = `-${file.stats.deletions}`; const annotatedHunkIndices = getAnnotatedHunkIndices(file); - const { filename, stateLabel } = fileLabelParts(file); return ( ) : null} - - {/* Clicking the file header jumps the main stream selection without collapsing to a single-file view. */} - - - {fitText(filename, Math.max(1, headerLabelWidth - (stateLabel?.length ?? 0)))} - - {stateLabel && {stateLabel}} - - - {additionsText} - - {deletionsText} - - + ) : null} - - - - {fitText(filename, Math.max(1, headerLabelWidth - (stateLabel?.length ?? 0)))} - - {stateLabel && {stateLabel}} - - - {additionsText} - - {deletionsText} - - + diff --git a/src/ui/components/scrollbar/VerticalScrollbar.tsx b/src/ui/components/scrollbar/VerticalScrollbar.tsx index ae2edce..80817c5 100644 --- a/src/ui/components/scrollbar/VerticalScrollbar.tsx +++ b/src/ui/components/scrollbar/VerticalScrollbar.tsx @@ -161,6 +161,7 @@ export const VerticalScrollbar = forwardRef {/* Track background */} diff --git a/src/ui/lib/sectionLayout.ts b/src/ui/lib/sectionLayout.ts new file mode 100644 index 0000000..4b9fc70 --- /dev/null +++ b/src/ui/lib/sectionLayout.ts @@ -0,0 +1,78 @@ +import type { DiffFile } from "../../core/types"; + +/** Stream geometry for one file section in the main review pane. */ +export interface SectionLayoutMetric { + fileId: string; + sectionIndex: number; + sectionTop: number; + headerTop: number; + bodyTop: number; + bodyHeight: number; + sectionBottom: number; +} + +/** Build absolute section offsets from file order and measured body heights. */ +export function buildSectionLayoutMetrics(files: DiffFile[], bodyHeights: number[]) { + const metrics: SectionLayoutMetric[] = []; + let cursor = 0; + + files.forEach((file, index) => { + const separatorHeight = index > 0 ? 1 : 0; + const bodyHeight = Math.max(0, bodyHeights[index] ?? 0); + const sectionTop = cursor; + const headerTop = sectionTop + separatorHeight; + const bodyTop = headerTop + 1; + const sectionBottom = bodyTop + bodyHeight; + + metrics.push({ + fileId: file.id, + sectionIndex: index, + sectionTop, + headerTop, + bodyTop, + bodyHeight, + sectionBottom, + }); + + cursor = sectionBottom; + }); + + return metrics; +} + +/** Return the file section that owns the viewport top, switching at each next header row. */ +export function findHeaderOwningSection(sectionMetrics: SectionLayoutMetric[], scrollTop: number) { + if (sectionMetrics.length === 0) { + return null; + } + + // Choose the last header whose top has reached the viewport, so separator rows still belong + // to the previous section until the next header itself takes over. + let low = 0; + let high = sectionMetrics.length - 1; + let winner = 0; + + while (low <= high) { + const mid = (low + high) >>> 1; + const metric = sectionMetrics[mid]!; + + if (metric.headerTop <= scrollTop) { + winner = mid; + low = mid + 1; + } else { + high = mid - 1; + } + } + + return sectionMetrics[winner]!; +} + +/** Resolve the scroll target needed to make one file header own the viewport top. */ +export function resolveFileHeaderTop(sectionMetrics: SectionLayoutMetric[], fileId: string) { + const targetSection = sectionMetrics.find((metric) => metric.fileId === fileId); + if (!targetSection) { + return null; + } + + return targetSection.headerTop; +} diff --git a/test/app-interactions.test.tsx b/test/app-interactions.test.tsx index 55354d7..bc26120 100644 --- a/test/app-interactions.test.tsx +++ b/test/app-interactions.test.tsx @@ -68,6 +68,17 @@ function createDiffFile( }; } +function lines(...values: string[]) { + return `${values.join("\n")}\n`; +} + +function createNumberedAssignmentLines(start: number, count: number, valueOffset = 0) { + return Array.from({ length: count }, (_, index) => { + const lineNumber = start + index; + return `export const line${String(lineNumber).padStart(2, "0")} = ${lineNumber + valueOffset};`; + }); +} + function createMockHostClient() { type Bridge = Parameters[0]; @@ -205,16 +216,8 @@ function createWrapBootstrap(pager = false): AppBootstrap { } function createLineScrollBootstrap(pager = false): AppBootstrap { - const before = - Array.from( - { length: 18 }, - (_, index) => `export const line${String(index + 1).padStart(2, "0")} = ${index + 1};`, - ).join("\n") + "\n"; - const after = - Array.from( - { length: 18 }, - (_, index) => `export const line${String(index + 1).padStart(2, "0")} = ${index + 101};`, - ).join("\n") + "\n"; + const before = lines(...createNumberedAssignmentLines(1, 18)); + const after = lines(...createNumberedAssignmentLines(1, 18, 100)); return { input: { @@ -292,17 +295,12 @@ function createDeepNoteBootstrap(): AppBootstrap { /** Build a long-line fixture that is tall enough to verify viewport-anchor restoration. */ function createWrapScrollBootstrap(): AppBootstrap { - const before = - Array.from( - { length: 18 }, - (_, index) => `export const line${String(index + 1).padStart(2, "0")} = ${index + 1};`, - ).join("\n") + "\n"; - const after = - Array.from( - { length: 18 }, - (_, index) => - `export const line${String(index + 1).padStart(2, "0")} = ${index + 101}; // this is intentionally long wrap coverage for viewport anchoring`, - ).join("\n") + "\n"; + const before = lines(...createNumberedAssignmentLines(1, 18)); + const after = lines( + ...createNumberedAssignmentLines(1, 18, 100).map( + (line) => `${line} // this is intentionally long wrap coverage for viewport anchoring`, + ), + ); return { input: { @@ -323,6 +321,44 @@ function createWrapScrollBootstrap(): AppBootstrap { }; } +function createTwoFileHunkBootstrap(): AppBootstrap { + const firstBeforeLines = createNumberedAssignmentLines(1, 16); + const secondBeforeLines = createNumberedAssignmentLines(17, 16); + + return { + input: { + kind: "git", + staged: false, + options: { + mode: "split", + }, + }, + changeset: { + id: "changeset:two-file-hunks", + sourceLabel: "repo", + title: "repo working tree", + files: [ + createDiffFile( + "first", + "first.ts", + lines(...firstBeforeLines), + lines(...createNumberedAssignmentLines(1, 16, 100)), + true, + ), + createDiffFile( + "second", + "second.ts", + lines(...secondBeforeLines), + lines(...createNumberedAssignmentLines(17, 16, 100)), + true, + ), + ], + }, + initialMode: "split", + initialTheme: "midnight", + }; +} + async function flush(setup: Awaited>) { await act(async () => { await setup.renderOnce(); @@ -1378,6 +1414,77 @@ describe("App interactions", () => { } }); + test("hunk navigation updates the pinned file header when jumping across files", async () => { + const setup = await testRender(, { + width: 220, + height: 10, + }); + + try { + await flush(setup); + + for (let index = 0; index < 10; index += 1) { + await act(async () => { + await setup.mockInput.pressArrow("down"); + }); + await flush(setup); + } + + let frame = setup.captureCharFrame(); + expect(frame).toContain("first.ts"); + + await act(async () => { + await setup.mockInput.typeText("]"); + }); + await flush(setup); + + frame = setup.captureCharFrame(); + expect(frame).toContain("second.ts"); + expect(frame).toContain("line17 = 117"); + } 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, + height: 10, + }); + + try { + await flush(setup); + + // Move partway into the first file so ownership can visibly change on sidebar selection. + for (let index = 0; index < 8; index += 1) { + await act(async () => { + await setup.mockInput.pressArrow("down"); + }); + await flush(setup); + } + + await act(async () => { + // Click inside the second file row in the left sidebar. + await setup.mockMouse.click(6, 4); + }); + await flush(setup); + + const frame = await waitForFrame( + setup, + (nextFrame) => + nextFrame.includes("second.ts") && (nextFrame.match(/first\.ts/g) ?? []).length === 1, + ); + expect(frame).toContain("second.ts"); + expect((frame.match(/first\.ts/g) ?? []).length).toBe(1); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("quit shortcuts route through the provided onQuit handler in regular and pager modes", async () => { const regularQuit = mock(() => undefined); const regularSetup = await testRender( diff --git a/test/integration/tuistory-hunk.integration.ts b/test/integration/tuistory-hunk.integration.ts index e04a3f9..d1a49fa 100644 --- a/test/integration/tuistory-hunk.integration.ts +++ b/test/integration/tuistory-hunk.integration.ts @@ -187,6 +187,53 @@ describe("Hunk integration via tuistory", () => { } }); + test("clicking a sidebar file pins that file header to the top 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"); + + for (let index = 0; index < 8; index += 1) { + await session.press("down"); + } + + const scrolled = await harness.waitForSnapshot( + session, + (text) => text.includes("line08 = 108") && text.includes("first.ts"), + 5_000, + ); + + expect(scrolled).toContain("first.ts"); + + await session.click(/M second\.ts\s+\+16 -16/); + const pinned = await harness.waitForSnapshot( + session, + (text) => + text.includes("second.ts") && + text.includes("line17 = 117") && + harness.countMatches(text, /first\.ts/g) === 1, + 5_000, + ); + + expect(pinned).toContain("second.ts"); + expect(pinned).toContain("line17 = 117"); + expect(harness.countMatches(pinned, /first\.ts/g)).toBe(1); + } finally { + session.close(); + } + }); + test("explicit split mode stays split after a live resize", async () => { const fixture = harness.createTwoFileRepoFixture(); const session = await harness.launchHunk({ diff --git a/test/integration/tuistoryHarness.ts b/test/integration/tuistoryHarness.ts index 1771248..7398e78 100644 --- a/test/integration/tuistoryHarness.ts +++ b/test/integration/tuistoryHarness.ts @@ -64,6 +64,14 @@ function writeText(path: string, content: string) { writeFileSync(path, content); } +/** Build numbered export lines so PTY fixtures can assert on stable visible content. */ +function createNumberedExportLines(start: number, count: number, valueOffset = 0) { + return Array.from({ length: count }, (_, index) => { + const lineNumber = start + index; + return `export const line${String(lineNumber).padStart(2, "0")} = ${lineNumber + valueOffset};`; + }).join("\n"); +} + function runGit(args: string[], cwd: string, allowExitCodeOne = false) { const proc = spawnSync("git", args, { cwd, @@ -224,6 +232,21 @@ export function createTuistoryHarness() { ]); } + function createPinnedHeaderRepoFixture() { + return createGitRepoFixture([ + { + path: "first.ts", + before: `${createNumberedExportLines(1, 16)}\n`, + after: `${createNumberedExportLines(1, 16, 100)}\n`, + }, + { + path: "second.ts", + before: `${createNumberedExportLines(17, 16)}\n`, + after: `${createNumberedExportLines(17, 16, 100)}\n`, + }, + ]); + } + function createSidebarJumpRepoFixture() { return createGitRepoFixture([ { @@ -342,6 +365,7 @@ export function createTuistoryHarness() { createLongWrapFilePair, createMultiHunkFilePair, createPagerPatchFixture, + createPinnedHeaderRepoFixture, createScrollableFilePair, createSidebarJumpRepoFixture, createTwoFileRepoFixture, diff --git a/test/ui-components.test.tsx b/test/ui-components.test.tsx index f9c32c7..97192b9 100644 --- a/test/ui-components.test.tsx +++ b/test/ui-components.test.tsx @@ -1,9 +1,11 @@ import { describe, expect, test } from "bun:test"; +import type { ScrollBoxRenderable } from "@opentui/core"; import { testRender } from "@opentui/react/test-utils"; import { parseDiffFromFile } from "@pierre/diffs"; import { act, createRef, type ReactNode } from "react"; import type { AppBootstrap, DiffFile } from "../src/core/types"; import { resolveTheme } from "../src/ui/themes"; +import { measureDiffSectionMetrics } from "../src/ui/lib/sectionHeights"; const { AppHost } = await import("../src/ui/AppHost"); const { buildSidebarEntries } = await import("../src/ui/lib/files"); @@ -161,6 +163,20 @@ function createWrappedViewportSizedBottomHunkDiffFile(id: string, path: string) return createDiffFile(id, path, lines(...beforeLines), lines(...afterLines)); } +function createTallDiffFile(id: string, path: string, count: number) { + const before = lines( + ...Array.from({ length: count }, (_, index) => `export const line${index + 1} = ${index + 1};`), + ); + const after = lines( + ...Array.from( + { length: count }, + (_, index) => `export const line${index + 1} = ${index + 1001};`, + ), + ); + + return createDiffFile(id, path, before, after); +} + function createDiffPaneProps( files: DiffFile[], theme = resolveTheme("midnight", null), @@ -200,6 +216,28 @@ function settleDiffPane(setup: Awaited>) { }); } +async function waitForFrame( + setup: Awaited>, + predicate: (frame: string) => boolean, + attempts = 8, +) { + let frame = setup.captureCharFrame(); + + for (let attempt = 0; attempt < attempts; attempt += 1) { + if (predicate(frame)) { + return frame; + } + + await act(async () => { + await Bun.sleep(50); + await setup.renderOnce(); + }); + frame = setup.captureCharFrame(); + } + + return frame; +} + function createBootstrap(): AppBootstrap { return { input: { @@ -468,6 +506,85 @@ describe("UI components", () => { } }); + test("DiffPane keeps the previous file header pinned until the next header reaches the top", async () => { + const theme = resolveTheme("midnight", null); + const firstFile = createTallDiffFile("first", "first.ts", 18); + const secondFile = createTallDiffFile("second", "second.ts", 18); + 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: 10, + }); + + const firstBodyHeight = measureDiffSectionMetrics( + firstFile, + "split", + true, + theme, + [], + 88, + true, + false, + ).bodyHeight; + const secondHeaderTop = firstBodyHeight + 2; + const separatorTop = secondHeaderTop - 1; + const settleStickyScroll = async () => { + await act(async () => { + for (let iteration = 0; iteration < 6; iteration += 1) { + await Bun.sleep(60); + await setup.renderOnce(); + } + }); + }; + + try { + await settleDiffPane(setup); + + let frame = setup.captureCharFrame(); + expect((frame.match(/first\.ts/g) ?? []).length).toBe(1); + + await act(async () => { + scrollRef.current?.scrollTo(3); + }); + await settleStickyScroll(); + + frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("first.ts")); + expect(frame).toContain("first.ts"); + + await act(async () => { + scrollRef.current?.scrollTo(separatorTop); + }); + await settleStickyScroll(); + + frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("first.ts")); + expect(frame).toContain("first.ts"); + + await act(async () => { + scrollRef.current?.scrollTo(secondHeaderTop); + }); + 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"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane keeps a viewport-sized selected hunk fully visible when it fits", async () => { const theme = resolveTheme("midnight", null); const props = createDiffPaneProps(