diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index 98f69808b..c3b68b145 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -1,7 +1,7 @@ import { execFileSync } from "node:child_process"; import { readFile, unlink, writeFile } from "node:fs/promises"; import path from "node:path"; -import { type Page } from "@playwright/test"; +import { type Locator, type Page } from "@playwright/test"; import { buildHostWorkspaceRoute, buildSettingsSectionRoute } from "../../src/utils/host-routes"; import { test, expect } from "../support/fixtures"; import { daemonWsRoutePattern } from "../support/helpers/daemon-port"; @@ -63,6 +63,10 @@ async function failNextDiscardRequest(page: Page): Promise { const CHANGES_PREFERENCES_KEY = "@paseo:changes-preferences"; +function diffGroups(scope: Page | Locator, fileIndex = 0): Locator { + return scope.getByTestId(new RegExp(`^diff-file-${fileIndex}-group-\\d+$`)); +} + const BEFORE = `import { useLayoutEffect, useMemo, useRef, useState } from "react"; interface UseMountedTabSetInput { @@ -465,12 +469,12 @@ test("Changes switches between inline and full-tab navigation", async ({ page }) await expect(visiblePanel).toBeVisible(); await expect(visiblePanel.getByText("use-mounted-tab-set.ts", { exact: true })).toBeVisible(); await expect(visiblePanel).toContainText("zz-deleted.ts"); - await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); + await expect(diffGroups(visiblePanel).first()).toBeVisible(); await expect(page.getByTestId("workspace-file-pane")).toHaveCount(0); await visiblePanel.getByTestId("diff-file-0-toggle").click(); - await expect(visiblePanel.getByTestId("diff-file-0-body")).toHaveCount(0); + await expect(diffGroups(visiblePanel)).toHaveCount(0); await visiblePanel.getByTestId("diff-file-0-toggle").click(); - await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); + await expect(diffGroups(visiblePanel).first()).toBeVisible(); const workingDiffLayoutToggle = visiblePanel.getByTestId("working-diff-toggle-layout"); await expect(workingDiffLayoutToggle).toHaveAccessibleName("Switch to side-by-side diff"); await workingDiffLayoutToggle.click(); @@ -486,14 +490,12 @@ test("Changes switches between inline and full-tab navigation", async ({ page }) ); await page.keyboard.press("Escape"); await visiblePanel.getByTestId("working-diff-toggle-expand-all").click(); - await expect(visiblePanel.getByTestId(/^diff-file-\d+-body$/)).toHaveCount(0); + await expect(visiblePanel.getByTestId(/^diff-file-\d+-group-\d+$/)).toHaveCount(0); await visiblePanel.getByTestId("working-diff-toggle-expand-all").click(); - await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); + await expect(diffGroups(visiblePanel).first()).toBeVisible(); await page.getByTestId("explorer-content-area").getByTestId("diff-file-0-toggle").click(); - await expect( - page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), - ).toHaveCount(0); + await expect(diffGroups(page.getByTestId("explorer-content-area"))).toHaveCount(0); await expect(page.getByTestId(/^workspace-working-diff-close-/)).toHaveCount(1); await writeFile(path.join(workspace.repoPath, "src/use-mounted-tab-set.ts"), BEFORE); @@ -516,13 +518,9 @@ test("Changes switches between inline and full-tab navigation", async ({ page }) await changesTabToggle.click(); await expect(page.getByTestId(/^workspace-working-diff-close-/)).toHaveCount(0); - await expect( - page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), - ).toBeVisible(); + await expect(diffGroups(page.getByTestId("explorer-content-area")).first()).toBeVisible(); await page.getByTestId("explorer-content-area").getByTestId("diff-file-0-toggle").click(); - await expect( - page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), - ).toHaveCount(0); + await expect(diffGroups(page.getByTestId("explorer-content-area"))).toHaveCount(0); }); test("changes diff switches between flat and tree file lists", async ({ page }) => { @@ -585,7 +583,7 @@ test("changes diff switches between flat and tree file lists", async ({ page }) await page.getByRole("button", { name: "Collapse all" }).click(); await expect(page.getByTestId("diff-file-0")).toHaveCount(0); await page.getByRole("button", { name: "Expand all" }).click(); - await expect(page.getByTestId("diff-file-0-body")).toBeVisible(); + await expect(diffGroups(page).first()).toBeVisible(); await page.getByTestId("diff-folder-src-toggle").click(); await expect(page.getByTestId("diff-file-0")).toHaveCount(0); @@ -609,6 +607,90 @@ test("changes diff applies code size changes to gutter and code typography", asy await expectVisibleDiffRowsShareTypography(page); }); +const LARGE_DIFF_WIDE_LINE_INDEX = 900; + +async function createWorkspaceWithLargeUnwrappedDiff(): Promise { + const workspace = await createWorkspaceWithMountedTabDiff(); + const addedLines = Array.from({ length: 1_200 }, (_, index) => { + if (index === 0) return `\t\t\t\tconst tabbed_${index} = "tabbed";`; + if (index === LARGE_DIFF_WIDE_LINE_INDEX) { + return `const wide_${index} = "${"界".repeat(80)}";`; + } + return `const generated_${index} = ${index};`; + }).join("\n"); + await writeFile( + path.join(workspace.repoPath, "src/use-mounted-tab-set.ts"), + `${AFTER}${addedLines}\n`, + ); + return workspace; +} + +async function scrollToWideDiffLine(page: Page): Promise { + await page.getByTestId("git-diff-scroll").evaluate((element, lineIndex) => { + element.scrollTop = lineIndex * 18; + element.dispatchEvent(new Event("scroll", { bubbles: false })); + }, LARGE_DIFF_WIDE_LINE_INDEX); + const wideLine = page.getByText(new RegExp(`const wide_${LARGE_DIFF_WIDE_LINE_INDEX}`)); + await expect(wideLine).toHaveCount(1); + return wideLine; +} + +test("unwrapped virtualized rows use one horizontal file surface", async ({ page }) => { + const workspace = await createWorkspaceWithLargeUnwrappedDiff(); + await useUnwrappedDiffLines(page); + await openWorkspaceChanges(page, workspace); + + const horizontalSurface = page.getByTestId("diff-horizontal-scroll"); + await expect(horizontalSurface).toHaveCount(1); + await expect(page.getByTestId(/^diff-file-0-group-\d+-scroll$/)).toHaveCount(0); + const codeBefore = await page.getByTestId("diff-code-row-0").boundingBox(); + const gutterBefore = await page.getByTestId("diff-gutter-row-0").boundingBox(); + expect(codeBefore).not.toBeNull(); + expect(gutterBefore).not.toBeNull(); + await horizontalSurface.evaluate((element, left) => { + element.scrollLeft = left; + element.dispatchEvent(new Event("scroll", { bubbles: false })); + }, 120); + await expect.poll(() => horizontalSurface.evaluate((element) => element.scrollLeft)).toBe(120); + await expect + .poll(async () => (await page.getByTestId("diff-code-row-0").boundingBox())?.x ?? 0) + .toBeLessThanOrEqual(codeBefore!.x - 119); + await expect + .poll(async () => (await page.getByTestId("diff-gutter-row-0").boundingBox())?.x ?? 0) + .toBeCloseTo(gutterBefore!.x, 0); + const initiallyMountedGroups = await readMountedDiffGroupIds(page); + await page.getByTestId("git-diff-scroll").evaluate((element) => { + element.scrollTop = Math.max(0, element.scrollHeight - element.clientHeight); + element.dispatchEvent(new Event("scroll", { bubbles: false })); + }); + await expect + .poll(async () => { + const mountedGroups = await readMountedDiffGroupIds(page); + return mountedGroups.some((group) => !initiallyMountedGroups.includes(group)); + }) + .toBe(true); + await expect.poll(() => horizontalSurface.evaluate((element) => element.scrollLeft)).toBe(120); + await expect(page.getByTestId(/^diff-file-0-group-\d+-scroll$/)).toHaveCount(0); + + const wideLine = await scrollToWideDiffLine(page); + await horizontalSurface.evaluate((element) => { + element.scrollLeft = element.scrollWidth; + element.dispatchEvent(new Event("scroll", { bubbles: false })); + }); + await expect + .poll(async () => { + const surfaceBounds = await horizontalSurface.boundingBox(); + const wideTextRight = await wideLine.evaluate((element) => { + const range = document.createRange(); + range.selectNodeContents(element); + return range.getBoundingClientRect().right; + }); + if (!surfaceBounds) return false; + return wideTextRight <= surfaceBounds.x + surfaceBounds.width + 1; + }) + .toBe(true); +}); + async function useCodeFont(page: Page, codeFontSize: number): Promise { await page.addInitScript( ({ settingsKey, fontSize }) => { @@ -634,6 +716,12 @@ async function useCodeFont(page: Page, codeFontSize: number): Promise { ); } +async function readMountedDiffGroupIds(page: Page): Promise<(string | null)[]> { + return diffGroups(page).evaluateAll((groups) => + groups.map((group) => group.getAttribute("data-testid")), + ); +} + async function useUnwrappedDiffLines(page: Page): Promise { await page.addInitScript( ({ preferencesKey }) => { @@ -682,21 +770,22 @@ async function readVisibleDiffRowGeometry(page: Page): Promise<{ codeLineHeight: number; }[]; }> { - return page.locator("body").evaluate(({ ownerDocument }) => { - const root = ownerDocument.querySelector('[data-testid="diff-file-0-body"]'); - if (!root) { + return diffGroups(page).evaluateAll((groups) => { + if (groups.length === 0) { throw new Error("Expanded diff body is not mounted"); } const readRows = (prefix: string, textPrefix: string) => - Array.from(root.querySelectorAll(`[data-testid^="${prefix}"]`)).map((row) => { - const testId = row.getAttribute("data-testid") ?? ""; - const index = Number(testId.slice(prefix.length)); - const rect = row.getBoundingClientRect(); - const text = root.querySelector(`[data-testid="${textPrefix}${index}"]`); - const lineHeight = text ? Number.parseFloat(getComputedStyle(text).lineHeight) : 0; - return { index, top: rect.top, height: rect.height, lineHeight }; - }); + groups.flatMap((group) => + Array.from(group.querySelectorAll(`[data-testid^="${prefix}"]`)).map((row) => { + const testId = row.getAttribute("data-testid") ?? ""; + const index = Number(testId.slice(prefix.length)); + const rect = row.getBoundingClientRect(); + const text = row.querySelector(`[data-testid="${textPrefix}${index}"]`); + const lineHeight = text ? Number.parseFloat(getComputedStyle(text).lineHeight) : 0; + return { index, top: rect.top, height: rect.height, lineHeight }; + }), + ); const gutters = new Map( readRows("diff-gutter-row-", "diff-gutter-text-").map((row) => [row.index, row]), @@ -800,7 +889,7 @@ async function openChangesInVisibleExplorer(page: Page): Promise { } async function expectExpandedMountedTabDiff(page: Page): Promise { - await expect(page.getByTestId("diff-file-0-body")).toBeVisible({ timeout: 30_000 }); + await expect(diffGroups(page).first()).toBeVisible({ timeout: 30_000 }); await expect(page.getByText("function createInitialMountedTabIds")).toBeVisible({ timeout: 30_000, }); @@ -839,15 +928,9 @@ async function returnToWorkspaceChanges(page: Page): Promise { } async function scrollToLowerUnwrappedDiffRows(page: Page): Promise { - const lastRowIndex = await page.getByTestId("diff-file-0-body").evaluate((root) => { - const rows = Array.from(root.querySelectorAll('[data-testid^="diff-code-row-"]')); - if (rows.length === 0) { - throw new Error("No unwrapped code rows are mounted"); - } - return Math.max( - ...rows.map((row) => Number((row.getAttribute("data-testid") ?? "").slice(14))), - ); - }); - await page.getByTestId(`diff-code-row-${lastRowIndex}`).scrollIntoViewIfNeeded(); - await expect(page.getByTestId(`diff-code-row-${lastRowIndex}`)).toBeVisible(); + const groups = diffGroups(page); + await expect.poll(() => groups.count()).toBeGreaterThan(1); + const lastMountedGroup = groups.last(); + await lastMountedGroup.scrollIntoViewIfNeeded(); + await expect(lastMountedGroup.locator('[data-testid^="diff-code-row-"]').last()).toBeVisible(); } diff --git a/packages/app/e2e/browser/commit-diff-panel.spec.ts b/packages/app/e2e/browser/commit-diff-panel.spec.ts index 1f658fac0..aa416bb11 100644 --- a/packages/app/e2e/browser/commit-diff-panel.spec.ts +++ b/packages/app/e2e/browser/commit-diff-panel.spec.ts @@ -55,7 +55,7 @@ test("commit history shows dates and shares diff layout preferences", async ({ await layoutToggle.click(); await expect(layoutToggle).toHaveAccessibleName("Switch to unified diff"); await expect(panel.getByTestId("diff-code-row-0")).toHaveCount(0); - await expect(panel.getByTestId("diff-file-0-body")).toBeVisible(); + await expect(panel.getByTestId(/^diff-file-0-group-\d+$/).first()).toBeVisible(); await page.getByTestId(/^workspace-commit-diff-close-/).click(); await expect(panel).toHaveCount(0); diff --git a/packages/app/src/components/diff-scroll.tsx b/packages/app/src/components/diff-scroll.tsx index 49110c3c7..d6e3a2a8d 100644 --- a/packages/app/src/components/diff-scroll.tsx +++ b/packages/app/src/components/diff-scroll.tsx @@ -1,58 +1,75 @@ -import { useState, useCallback, useEffect, useId, useRef } from "react"; +import { useState, useCallback, useEffect, useId } from "react"; +import { type LayoutChangeEvent, type StyleProp, type ViewStyle } from "react-native"; +import { ScrollView } from "react-native-gesture-handler"; import { - type LayoutChangeEvent, - type NativeSyntheticEvent, - type NativeScrollEvent, - type StyleProp, - type ViewStyle, -} from "react-native"; -import { ScrollView, type ScrollView as ScrollViewType } from "react-native-gesture-handler"; + createAnimatedComponent, + useAnimatedScrollHandler, + useSharedValue, + type SharedValue, +} from "react-native-reanimated"; +import { scheduleOnRN } from "react-native-worklets"; import { useHorizontalScrollOptional } from "@/contexts/horizontal-scroll-context"; import { useFileExplorerCloseGestureRef } from "@/mobile-panels/gestures"; +const AnimatedScrollView = createAnimatedComponent(ScrollView); + interface DiffScrollProps { children: React.ReactNode; - scrollViewWidth: number; onScrollViewWidthChange: (width: number) => void; style?: StyleProp; contentContainerStyle?: StyleProp; + offset: SharedValue; + testID?: string; } export function DiffScroll({ children, - scrollViewWidth: _scrollViewWidth, onScrollViewWidthChange, style, contentContainerStyle, + offset, + testID, }: DiffScrollProps) { - const [isAtLeftEdge, setIsAtLeftEdge] = useState(true); const horizontalScroll = useHorizontalScrollOptional(); const scrollId = useId(); - const scrollViewRef = useRef(null); + const [isAtLeftEdge, setIsAtLeftEdge] = useState(true); + const isAtLeftEdgeValue = useSharedValue(true); const closeGestureRef = useFileExplorerCloseGestureRef(); // Register/unregister scroll offset tracking useEffect(() => { if (!horizontalScroll) return; - // Start at 0 (not scrolled) horizontalScroll.registerScrollOffset(scrollId, 0); return () => { horizontalScroll.unregisterScrollOffset(scrollId); }; }, [horizontalScroll, scrollId]); - const handleScroll = useCallback( - (event: NativeSyntheticEvent) => { - const offsetX = event.nativeEvent.contentOffset.x; - // Track if we're at the left edge (with small threshold for float precision) - setIsAtLeftEdge(offsetX <= 1); + const recordScrollOffset = useCallback( + (offsetX: number) => { if (horizontalScroll) { horizontalScroll.registerScrollOffset(scrollId, offsetX); } }, [horizontalScroll, scrollId], ); + const recordLeftEdge = useCallback((atLeftEdge: boolean) => { + setIsAtLeftEdge(atLeftEdge); + }, []); + + const handleScroll = useAnimatedScrollHandler({ + onScroll: (event) => { + const offsetX = event.contentOffset.x; + offset.value = offsetX; + const atLeftEdge = offsetX <= 1; + if (atLeftEdge !== isAtLeftEdgeValue.value) { + isAtLeftEdgeValue.value = atLeftEdge; + scheduleOnRN(recordLeftEdge, atLeftEdge); + } + scheduleOnRN(recordScrollOffset, offsetX); + }, + }); const handleLayout = useCallback( (e: LayoutChangeEvent) => onScrollViewWidthChange(e.nativeEvent.layout.width), @@ -60,8 +77,7 @@ export function DiffScroll({ ); return ( - {children} - + ); } diff --git a/packages/app/src/components/diff-scroll.web.tsx b/packages/app/src/components/diff-scroll.web.tsx index 4caa3ac40..22cca0856 100644 --- a/packages/app/src/components/diff-scroll.web.tsx +++ b/packages/app/src/components/diff-scroll.web.tsx @@ -1,12 +1,23 @@ -import { useCallback } from "react"; -import { ScrollView, type LayoutChangeEvent, type StyleProp, type ViewStyle } from "react-native"; +import { useCallback, useId, useLayoutEffect, useState } from "react"; +import { + type LayoutChangeEvent, + type NativeScrollEvent, + type NativeSyntheticEvent, + type StyleProp, + type ViewStyle, +} from "react-native"; +import { ScrollView } from "react-native-gesture-handler"; +import type { SharedValue } from "react-native-reanimated"; +import { useHorizontalScrollOptional } from "@/contexts/horizontal-scroll-context"; +import { useFileExplorerCloseGestureRef } from "@/mobile-panels/gestures"; interface DiffScrollProps { children: React.ReactNode; - scrollViewWidth: number; onScrollViewWidthChange: (width: number) => void; style?: StyleProp; contentContainerStyle?: StyleProp; + offset: SharedValue; + testID?: string; } export function DiffScroll({ @@ -14,9 +25,31 @@ export function DiffScroll({ onScrollViewWidthChange, style, contentContainerStyle, + offset, + testID, }: DiffScrollProps) { + const horizontalScroll = useHorizontalScrollOptional(); + const [isAtLeftEdge, setIsAtLeftEdge] = useState(true); + const scrollId = useId(); + const closeGestureRef = useFileExplorerCloseGestureRef(); + + useLayoutEffect(() => { + horizontalScroll?.registerScrollOffset(scrollId, 0); + return () => horizontalScroll?.unregisterScrollOffset(scrollId); + }, [horizontalScroll, scrollId]); + + const handleScroll = useCallback( + (event: NativeSyntheticEvent) => { + const offsetX = event.nativeEvent.contentOffset.x; + offset.value = offsetX; + setIsAtLeftEdge(offsetX <= 1); + horizontalScroll?.registerScrollOffset(scrollId, offsetX); + }, + [horizontalScroll, offset, scrollId], + ); + const handleLayout = useCallback( - (e: LayoutChangeEvent) => onScrollViewWidthChange(e.nativeEvent.layout.width), + (event: LayoutChangeEvent) => onScrollViewWidthChange(event.nativeEvent.layout.width), [onScrollViewWidthChange], ); @@ -25,9 +58,14 @@ export function DiffScroll({ horizontal nestedScrollEnabled showsHorizontalScrollIndicator + bounces={false} style={style} contentContainerStyle={contentContainerStyle} + onScroll={handleScroll} + scrollEventThrottle={16} onLayout={handleLayout} + testID={testID} + waitFor={isAtLeftEdge && closeGestureRef?.current ? closeGestureRef : undefined} > {children} diff --git a/packages/app/src/git/diff-flat-items.test.ts b/packages/app/src/git/diff-flat-items.test.ts deleted file mode 100644 index a3df76492..000000000 --- a/packages/app/src/git/diff-flat-items.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { buildDiffFlatItems, sumHeightsBefore, type DiffFlatItem } from "./diff-flat-items"; -import type { ParsedDiffFile } from "@/git/use-diff-query"; - -function createFile(path: string, additions = 1, deletions = 0): ParsedDiffFile { - return { path, isNew: false, isDeleted: false, additions, deletions, hunks: [] }; -} - -function summarize(items: DiffFlatItem[]): string[] { - return items.map((item) => { - if (item.type === "folder") { - return `${" ".repeat(item.depth)}[${item.displayName}]${item.collapsed ? " (collapsed)" : ""}`; - } - const base = item.file.path.split("/").pop(); - return `${" ".repeat(item.depth)}${item.type === "body" ? "body:" : ""}${base}`; - }); -} - -describe("buildDiffFlatItems", () => { - const files = [createFile("src/app/a.ts"), createFile("src/app/nested/b.ts")]; - - it("emits a body and a sticky index for each expanded file", () => { - const { items, stickyHeaderIndices } = buildDiffFlatItems({ - files: [createFile("a.ts"), createFile("b.ts")], - viewMode: "tree", - collapsedFolders: new Set(), - expandedPaths: new Set(["a.ts"]), - }); - // Root-level files (no folder): header, body (for expanded a.ts), header - expect(summarize(items)).toEqual(["a.ts", "body:a.ts", "b.ts"]); - // sticky points at the header (index 0), not the body - expect(stickyHeaderIndices).toEqual([0]); - expect(items[0].type).toBe("header"); - }); - - it("emits the old flat list without folder rows or indentation", () => { - const { items, stickyHeaderIndices } = buildDiffFlatItems({ - files, - viewMode: "flat", - collapsedFolders: new Set(), - expandedPaths: new Set(["src/app/a.ts"]), - }); - - expect(summarize(items)).toEqual(["a.ts", "body:a.ts", "b.ts"]); - expect(stickyHeaderIndices).toEqual([0]); - expect(items.every((item) => item.type !== "folder")).toBe(true); - }); - - it("groups files under compressed folder rows, all expanded by default", () => { - const { items } = buildDiffFlatItems({ - files, - viewMode: "tree", - collapsedFolders: new Set(), - expandedPaths: new Set(), - }); - // dirs sort before files within a level: [nested] precedes a.ts - expect(summarize(items)).toEqual(["[src/app]", " [nested]", " b.ts", " a.ts"]); - }); - - it("collapsing a folder hides its descendants but keeps the row", () => { - const { items } = buildDiffFlatItems({ - files, - viewMode: "tree", - collapsedFolders: new Set(["src/app/nested"]), - expandedPaths: new Set(), - }); - expect(summarize(items)).toEqual(["[src/app]", " [nested] (collapsed)", " a.ts"]); - }); - - it("collapsing an ancestor hides everything below it", () => { - const { items } = buildDiffFlatItems({ - files, - viewMode: "tree", - collapsedFolders: new Set(["src/app"]), - expandedPaths: new Set(), - }); - expect(summarize(items)).toEqual(["[src/app] (collapsed)"]); - }); - - it("derives sticky indices file-headers-only from the post-collapse list", () => { - // dirs-first: [src/app], [nested], b.ts, a.ts. Expanding a.ts puts its - // header at index 3, with the body right after. - const { items, stickyHeaderIndices } = buildDiffFlatItems({ - files, - viewMode: "tree", - collapsedFolders: new Set(), - expandedPaths: new Set(["src/app/a.ts"]), - }); - expect(summarize(items)).toEqual([ - "[src/app]", - " [nested]", - " b.ts", - " a.ts", - " body:a.ts", - ]); - expect(stickyHeaderIndices).toEqual([3]); - for (const idx of stickyHeaderIndices) { - expect(items[idx].type).toBe("header"); - } - }); - - it("maps tree file rows back to their original files array index", () => { - const { items } = buildDiffFlatItems({ - files, - viewMode: "tree", - collapsedFolders: new Set(), - expandedPaths: new Set(), - }); - // Tree order is b.ts (fileIndex 1) then a.ts (fileIndex 0) - const headers = items.filter((i) => i.type === "header"); - expect(headers.map((h) => (h.type === "header" ? h.fileIndex : -1))).toEqual([1, 0]); - }); -}); - -describe("sumHeightsBefore", () => { - const items = buildDiffFlatItems({ - files: [createFile("src/a.ts"), createFile("src/b.ts")], - viewMode: "tree", - collapsedFolders: new Set(), - expandedPaths: new Set(), - }).items; // [folder, a.ts, b.ts] - - const heightFor = (item: DiffFlatItem) => (item.type === "folder" ? 10 : 20); - - it("sums the heights of items before the index, counting folder rows", () => { - expect(sumHeightsBefore(items, 0, heightFor)).toBe(0); - expect(sumHeightsBefore(items, 1, heightFor)).toBe(10); // folder above a.ts - expect(sumHeightsBefore(items, 2, heightFor)).toBe(30); // folder + a.ts above b.ts - }); - - it("clamps an out-of-range index to the list length", () => { - expect(sumHeightsBefore(items, 999, heightFor)).toBe(50); - }); -}); diff --git a/packages/app/src/git/diff-flat-items.ts b/packages/app/src/git/diff-flat-items.ts deleted file mode 100644 index cd60dd97c..000000000 --- a/packages/app/src/git/diff-flat-items.ts +++ /dev/null @@ -1,121 +0,0 @@ -import type { ParsedDiffFile } from "@/git/use-diff-query"; -import { - buildDiffTree, - compressSingleChildChains, - flattenDiffTree, - type DiffTreeDirNode, -} from "@/git/diff-tree"; - -// The row model for the Changes FlatList. `fileIndex` is the file's index within -// the (path-sorted) `files` array — a stable identity for testIDs/keys that is -// independent of folder rows and collapse state. `depth` drives indentation. -export type DiffFlatItem = - | { - type: "folder"; - dirPath: string; - displayName: string; - depth: number; - collapsed: boolean; - additions: number; - deletions: number; - } - | { type: "header"; file: ParsedDiffFile; fileIndex: number; isExpanded: boolean; depth: number } - | { type: "body"; file: ParsedDiffFile; fileIndex: number; depth: number }; - -export interface DiffFlatItemsResult { - items: DiffFlatItem[]; - /** Indices into `items` of expanded file headers — folder rows are never sticky. */ - stickyHeaderIndices: number[]; -} - -export interface BuildDiffFlatItemsInput { - files: ParsedDiffFile[]; - viewMode: "flat" | "tree"; - /** Full uncompressed directory paths that are collapsed (empty = all expanded). */ - collapsedFolders: ReadonlySet; - /** File paths whose diff body is expanded. */ - expandedPaths: ReadonlySet; - /** - * Pre-built compressed tree (from the same `files`). Pass it to avoid rebuilding - * the tree on every collapse toggle; omitted callers (tests) build it internally. - */ - tree?: DiffTreeDirNode; -} - -/** - * Build the FlatList item model plus its sticky-header indices: files grouped - * under directory rows, single-child chains compressed, descendants of collapsed - * folders omitted. - * - * Sticky indices are derived from the FINAL (post-collapse) item list and only - * ever point at expanded FILE headers, so folder rows can't stack or collide in - * the sticky header stack. - */ -export function buildDiffFlatItems({ - files, - viewMode, - collapsedFolders, - expandedPaths, - tree, -}: BuildDiffFlatItemsInput): DiffFlatItemsResult { - const items: DiffFlatItem[] = []; - const stickyHeaderIndices: number[] = []; - - const pushFile = (file: ParsedDiffFile, fileIndex: number, depth: number): void => { - const isExpanded = expandedPaths.has(file.path); - items.push({ type: "header", file, fileIndex, isExpanded, depth }); - if (isExpanded) { - stickyHeaderIndices.push(items.length - 1); - items.push({ type: "body", file, fileIndex, depth }); - } - }; - - if (viewMode === "flat") { - for (const [fileIndex, file] of files.entries()) { - pushFile(file, fileIndex, 0); - } - return { items, stickyHeaderIndices }; - } - - const indexByPath = new Map(files.map((file, index) => [file.path, index])); - const compressedTree = tree ?? compressSingleChildChains(buildDiffTree(files)); - const rows = flattenDiffTree(compressedTree, collapsedFolders); - - for (const row of rows) { - if (row.kind === "folder") { - items.push({ - type: "folder", - dirPath: row.dirPath, - displayName: row.displayName, - depth: row.depth, - collapsed: collapsedFolders.has(row.dirPath), - additions: row.additions, - deletions: row.deletions, - }); - continue; - } - const fileIndex = indexByPath.get(row.file.path); - if (fileIndex === undefined) { - // Should never happen: the tree is built from the same `files` array. - continue; - } - pushFile(row.file, fileIndex, row.depth); - } - - return { items, stickyHeaderIndices }; -} - -/** Cumulative height of every item before `index`. Single source of truth for - * FlatList's getItemLayout AND the collapse scroll-anchor math. */ -export function sumHeightsBefore( - items: DiffFlatItem[], - index: number, - heightFor: (item: DiffFlatItem) => number, -): number { - let offset = 0; - const end = Math.min(index, items.length); - for (let i = 0; i < end; i++) { - offset += heightFor(items[i]); - } - return Math.max(0, offset); -} diff --git a/packages/app/src/git/diff-horizontal-surface.tsx b/packages/app/src/git/diff-horizontal-surface.tsx new file mode 100644 index 000000000..c16be414b --- /dev/null +++ b/packages/app/src/git/diff-horizontal-surface.tsx @@ -0,0 +1,80 @@ +import { createContext, useContext, useMemo, type ReactNode } from "react"; +import { type LayoutChangeEvent, type StyleProp, type ViewStyle, View } from "react-native"; +import Animated, { + useAnimatedStyle, + useSharedValue, + type SharedValue, +} from "react-native-reanimated"; +import { DiffScroll } from "@/components/diff-scroll"; + +const FILL: ViewStyle = { flex: 1 }; +const CLIP: ViewStyle = { flex: 1, overflow: "hidden" }; + +const DiffHorizontalOffsetContext = createContext | null>(null); + +export function DiffHorizontalSurface({ + children, + contentWidth, + viewportWidth, + onViewportWidthChange, +}: { + children: ReactNode; + contentWidth: number; + viewportWidth: number; + onViewportWidthChange: (width: number) => void; +}) { + const offset = useSharedValue(0); + const pinnedViewportStyle = useAnimatedStyle(() => ({ + transform: [{ translateX: offset.value }], + })); + const contentContainerStyle = useMemo>( + () => [{ flexGrow: 1 }, { minWidth: Math.max(viewportWidth, contentWidth) }], + [contentWidth, viewportWidth], + ); + return ( + + + + {children} + + + + ); +} + +export function DiffHorizontalContent({ + children, + viewportStyle, + contentStyle, + onContentLayout, + testID, +}: { + children: ReactNode; + viewportStyle?: StyleProp; + contentStyle?: StyleProp; + onContentLayout?: (event: LayoutChangeEvent) => void; + testID?: string; +}) { + const offset = useContext(DiffHorizontalOffsetContext); + const translatedContentStyle = useAnimatedStyle(() => ({ + transform: [{ translateX: -(offset?.value ?? 0) }], + })); + + return ( + + + {children} + + + ); +} diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index f0756b34d..a594faa9c 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -1,4 +1,5 @@ import { + Fragment, useState, useCallback, useEffect, @@ -42,7 +43,6 @@ import { WrapText, } from "lucide-react-native"; import { type ParsedDiffFile, type DiffLine, type HighlightToken } from "@/git/use-diff-query"; -import { buildDiffFlatItems, sumHeightsBefore, type DiffFlatItem } from "@/git/diff-flat-items"; import { buildDiffTree, collectDirPaths, compressSingleChildChains } from "@/git/diff-tree"; import { DiffFolderRow } from "@/git/diff-folder-row"; import { @@ -59,13 +59,10 @@ import { useCheckoutPrStatusQuery } from "@/git/use-pr-status-query"; import { CommitsSection } from "@/git/commits-section/commits-section"; import { useChangesPreferences } from "@/hooks/use-changes-preferences"; import { useAppSettings } from "@/hooks/use-settings"; -import { DiffScroll } from "@/components/diff-scroll"; import { syntaxTokenStyleFor } from "@/styles/syntax-token-styles"; import { CODE_SURFACE_DATASET } from "@/styles/code-surface"; import { shouldAnchorHeaderBeforeCollapse } from "@/git/diff-scroll"; import { - buildSplitDiffRows, - buildUnifiedDiffLines, type ReviewableDiffTarget, type SplitDiffDisplayLine, type SplitDiffRow, @@ -107,6 +104,7 @@ import { formatDiffContentText, formatDiffGutterText, hasVisibleDiffTokens, + compactHighlightTokens, } from "@/utils/diff-rendering"; import { isWeb, isNative } from "@/constants/platform"; import { useWorkspaceFileDragSource } from "@/attachments/use-workspace-file-drag-source"; @@ -122,6 +120,15 @@ import { import { usePublishWorkingDiffAttachment, useWorkingDiff } from "@/git/use-working-diff"; import { DiffTooLargeState } from "@/git/diff-too-large-state"; import { openDesktopTarget, useDesktopOpenTargets } from "@/workspace/desktop-open-targets"; +import { + buildDiffRowIndex, + createDiffRowCache, + DIFF_VIRTUALIZATION_CONFIG, + type DiffListItem, + type SplitRowGroupItem, + type UnifiedRowGroupItem, +} from "@/git/diff-rows/model"; +import { DiffHorizontalContent, DiffHorizontalSurface } from "@/git/diff-horizontal-surface"; export type { GitActionId, GitAction, GitActions } from "@/git/policy"; @@ -176,10 +183,6 @@ function useDiffRowMetricsStyle(textMetricsStyle: TextStyle): StyleProp{token.text}; -} - function HighlightedText({ tokens, textMetricsStyle, @@ -196,15 +199,21 @@ function HighlightedText({ [textMetricsStyle, wrapLines], ); - const keyedTokens = useMemo( - () => tokens.map((token, index) => ({ key: `${index}-${token.text}`, token })), - [tokens], - ); + const keyedTokens = useMemo(() => { + let characterOffset = 0; + return compactHighlightTokens(tokens).map((token) => { + const keyedToken = { key: `${characterOffset}-${token.style ?? "base"}`, token }; + characterOffset += token.text.length; + return keyedToken; + }); + }, [tokens]); return ( {keyedTokens.map(({ key, token }) => ( - + + {token.text} + ))} ); @@ -299,6 +308,7 @@ function LongPressableLine({ hoverTargetKey, onHoverTargetChange, style, + testID, children, }: { reviewTarget: ReviewableDiffTarget | null | undefined; @@ -307,6 +317,7 @@ function LongPressableLine({ hoverTargetKey?: string | null; onHoverTargetChange?: (key: string | null) => void; style: StyleProp; + testID?: string; children: ReactNode; }) { const onStartComment = reviewActions?.onStartComment; @@ -336,17 +347,26 @@ function LongPressableLine({ if (isWeb && (onHoverChange || onHoverTargetChange)) { return ( - + {children} ); } if (!isNative || !reviewTarget || !onStartComment) { - return {children}; + return ( + + {children} + + ); } return ( - + {children} ); @@ -371,6 +391,7 @@ function DiffGutterCell({ style, textTestID, actionTestID, + rowTestID, }: { lineNumber: number | null; type: DiffLine["type"] | undefined | null; @@ -382,6 +403,7 @@ function DiffGutterCell({ style?: StyleProp; textTestID?: string; actionTestID?: string; + rowTestID?: string; }) { const lineHeight = getNumericLineHeight(textMetricsStyle); const rowMetricsStyle = useDiffRowMetricsStyle(textMetricsStyle); @@ -425,6 +447,7 @@ function DiffGutterCell({ onStartComment={onStartComment} style={containerStyle} actionTestID={actionTestID} + testID={rowTestID} > {formatDiffGutterText(lineNumber)} @@ -443,6 +466,7 @@ function DiffTextLine({ hoverTargetKey, onHoverTargetChange, textTestID, + rowTestID, }: { line: DiffLine; wrapLines: boolean; @@ -453,6 +477,7 @@ function DiffTextLine({ hoverTargetKey?: string | null; onHoverTargetChange?: (key: string | null) => void; textTestID?: string; + rowTestID?: string; }) { const visibleTokens = hasVisibleDiffTokens(line.tokens) ? line.tokens : null; const rowMetricsStyle = useDiffRowMetricsStyle(textMetricsStyle); @@ -483,6 +508,7 @@ function DiffTextLine({ hoverTargetKey={hoverTargetKey} onHoverTargetChange={onHoverTargetChange} style={containerStyle} + testID={rowTestID} > {line.type !== "header" && visibleTokens ? ( (null); const wrapCellStyle = useMemo( @@ -835,16 +868,19 @@ function SplitDiffColumn({ const linesContainerRowStyle = useMemo( () => [ styles.linesContainer, - scrollWidth > 0 && inlineUnistylesStyle({ minWidth: scrollWidth }), + inlineUnistylesStyle({ minWidth: Math.max(textViewportWidth, minContentWidth) }), ], - [scrollWidth], + [minContentWidth, textViewportWidth], ); const headerLineTextStyle = useMemo( () => [styles.diffTextMetrics, textMetricsStyle, styles.diffLineText, styles.headerLineText], [textMetricsStyle], ); - const keyedRows = useMemo(() => rows.map((row, i) => ({ key: `row-${i}`, row })), [rows]); + const keyedRows = useMemo( + () => rows.map((row, index) => ({ key: `row-${startRowIndex + index}`, row })), + [rows, startRowIndex], + ); if (wrapLines) { return ( @@ -932,50 +968,47 @@ function SplitDiffColumn({ ); })} - - - {keyedRows.map(({ key, row }) => { - if (row.kind === "header") { - return ( - - {row.content} - - ); - } - const line = side === "left" ? row.left : row.right; - const reviewTargetKey = line?.reviewTarget?.key ?? null; - const reviewRowState = getSplitInlineReviewThreadState({ - left: row.left?.reviewTarget, - right: row.right?.reviewTarget, - reviewActions, - }); + {keyedRows.map(({ key, row }) => { + if (row.kind === "header") { return ( - - - + + {row.content} ); - })} - - + } + const line = side === "left" ? row.left : row.right; + const reviewTargetKey = line?.reviewTarget?.key ?? null; + const reviewRowState = getSplitInlineReviewThreadState({ + left: row.left?.reviewTarget, + right: row.right?.reviewTarget, + reviewActions, + }); + return ( + + + + + ); + })} + ); } @@ -1254,194 +1287,251 @@ const DiffFileHeader = memo(function DiffFileHeader({ ); }); -export function DiffFileBody({ - file, - layout, - wrapLines, - codeFontSize, - textMetricsStyle, - reviewActions, - onBodyHeightChange, - testID, -}: { - file: ParsedDiffFile; - layout: "unified" | "split"; +function maxLineNumber(file: ParsedDiffFile): number { + let maximum = 0; + for (const hunk of file.hunks) { + maximum = Math.max(maximum, hunk.oldStart + hunk.oldCount, hunk.newStart + hunk.newCount); + } + return maximum; +} + +function DiffFileStatus({ file }: { file: ParsedDiffFile }) { + const { t } = useTranslation(); + return ( + + + + {file.status === "binary" + ? t("workspace.git.diff.binaryFile") + : t("workspace.git.diff.tooLarge")} + + + + ); +} + +interface DiffRowGroupProps { + item: UnifiedRowGroupItem | SplitRowGroupItem; + viewportWidth: number; wrapLines: boolean; codeFontSize: number; textMetricsStyle: TextStyle; reviewActions?: InlineReviewActions; - onBodyHeightChange?: (file: ParsedDiffFile, height: number) => void; - testID?: string; -}) { - const [scrollViewWidth, setScrollViewWidth] = useState(0); - const [bodyWidth, setBodyWidth] = useState(0); + onHeightChange?: (key: string, height: number) => void; +} + +function DiffRowGroupView({ + item, + viewportWidth, + wrapLines, + codeFontSize, + textMetricsStyle, + reviewActions, + onHeightChange, +}: DiffRowGroupProps) { const [hoveredReviewTargetKey, setHoveredReviewTargetKey] = useState(null); - const { t } = useTranslation(); const handleLayout = useCallback( (event: LayoutChangeEvent) => { - setBodyWidth(event.nativeEvent.layout.width); - onBodyHeightChange?.(file, event.nativeEvent.layout.height); + if (wrapLines) { + onHeightChange?.(item.key, event.nativeEvent.layout.height); + } }, - [file, onBodyHeightChange], + [item.key, onHeightChange, wrapLines], ); - const availableWidth = bodyWidth > 0 ? bodyWidth : scrollViewWidth; + const minimumContentWidth = Math.ceil(item.contentColumns * codeFontSize * 0.75 + SPACING[4]); + const gutterWidth = lineNumberGutterWidth(maxLineNumber(item.file), codeFontSize); + const textViewportWidth = Math.max(0, viewportWidth - gutterWidth); + const minimumTextWidth = Math.max(textViewportWidth, minimumContentWidth); const linesContainerRowStyle = useMemo( - () => [ - styles.linesContainer, - availableWidth > 0 && inlineUnistylesStyle({ minWidth: availableWidth }), - ], - [availableWidth], + () => [styles.linesContainer, inlineUnistylesStyle({ minWidth: minimumTextWidth })], + [minimumTextWidth], ); + const groupStyle = useMemo( + () => [ + styles.fileSectionBodyContainer, + item.isLast && styles.fileSectionBorder, + item.isFirst && styles.diffContent, + styles.diffGroupContent, + ], + [item.isFirst, item.isLast], + ); + + if (item.type === "split_group") { + return ( + + + + + ); + } + + if (wrapLines) { + return ( + + + {item.lines.map(({ line, lineNumber, key, reviewTarget }, localIndex) => { + const index = item.startRowIndex + localIndex; + return ( + + + + + ); + })} + + + ); + } return ( - {(() => { - if (file.status === "too_large" || file.status === "binary") { + + {item.lines.map(({ line, lineNumber, key, reviewTarget }, localIndex) => { + const index = item.startRowIndex + localIndex; + const hasReviewThread = + getInlineReviewThreadState({ reviewTarget, reviewActions }) !== null; return ( - - - {file.status === "binary" - ? t("workspace.git.diff.binaryFile") - : t("workspace.git.diff.tooLarge")} - - - ); - } - - let maxLineNo = 0; - for (const hunk of file.hunks) { - maxLineNo = Math.max( - maxLineNo, - hunk.oldStart + hunk.oldCount, - hunk.newStart + hunk.newCount, - ); - } - const gutterWidth = lineNumberGutterWidth(maxLineNo, codeFontSize); - - if (layout === "split") { - const rows = buildSplitDiffRows(file); - return ( - - + - - + {hasReviewThread ? ( + + ) : null} + ); - } - - const computedLines = buildUnifiedDiffLines(file); - - if (wrapLines) { + })} + + + {item.lines.map(({ line, key, reviewTarget }, localIndex) => { + const index = item.startRowIndex + localIndex; + const hasReviewThread = + getInlineReviewThreadState({ reviewTarget, reviewActions }) !== null; return ( - - - {computedLines.map(({ line, lineNumber, key, reviewTarget }, index) => ( - - - - - ))} - - + + + {hasReviewThread ? ( + + ) : null} + ); - } - - const textViewportWidth = - scrollViewWidth > 0 ? scrollViewWidth : Math.max(0, bodyWidth - gutterWidth); - return ( - - - {computedLines.map(({ line, lineNumber, key, reviewTarget }, index) => ( - - - - - ))} - - - - {computedLines.map(({ line, key, reviewTarget }, index) => ( - - - - - ))} - - - - ); - })()} + })} + ); } +function areDiffRowGroupPropsEqual(previous: DiffRowGroupProps, next: DiffRowGroupProps): boolean { + // Row arrays are rebuilt with the index. File identity pins cached row content, while the + // geometry key and compared grouping inputs pin group membership. + return ( + previous.item.key === next.item.key && + previous.item.type === next.item.type && + previous.item.file === next.item.file && + previous.item.fileIndex === next.item.fileIndex && + previous.item.depth === next.item.depth && + previous.item.groupIndex === next.item.groupIndex && + previous.item.startRowIndex === next.item.startRowIndex && + previous.item.isFirst === next.item.isFirst && + previous.item.isLast === next.item.isLast && + previous.item.contentColumns === next.item.contentColumns && + previous.viewportWidth === next.viewportWidth && + previous.wrapLines === next.wrapLines && + previous.codeFontSize === next.codeFontSize && + previous.textMetricsStyle === next.textMetricsStyle && + previous.reviewActions === next.reviewActions && + previous.onHeightChange === next.onHeightChange + ); +} + +const DiffRowGroup = memo(DiffRowGroupView, areDiffRowGroupPropsEqual); + interface GitDiffPaneProps { serverId: string; workspaceId?: string | null; @@ -1824,42 +1914,15 @@ export function DiffOptionsMenu({ const ThemedRotateCw = withUnistyles(RotateCw); -type DiffFlatItemLayoutGetter = NonNullable["getItemLayout"]>; +type DiffListItemLayoutGetter = NonNullable["getItemLayout"]>; + +// COMPAT(react-native-virtualized-list-bounded-data-growth): RN 0.81 carries the old index range +// through middle insertions, synchronously mounting every inserted diff group in that range. +const BOUNDED_DIFF_LIST_DATA_GROWTH_PROPS = { + unstable_boundedRangeOnDataChange: true, +} as const; const EMPTY_PATH_LIST: string[] = []; - -interface DiffFileMetrics { - contentLength: number; - splitLineCount?: number; - unifiedLineCount: number; -} - -const diffFileMetricsCache = new WeakMap(); - -function getDiffFileMetrics(file: ParsedDiffFile): DiffFileMetrics { - const cached = diffFileMetricsCache.get(file); - if (cached) { - return cached; - } - let contentLength = 0; - let unifiedLineCount = 0; - for (const hunk of file.hunks) { - unifiedLineCount += hunk.lines.length; - for (const line of hunk.lines) { - contentLength += line.content.length; - } - } - const metrics = { contentLength, unifiedLineCount }; - diffFileMetricsCache.set(file, metrics); - return metrics; -} - -function getSplitDiffLineCount(file: ParsedDiffFile): number { - const metrics = getDiffFileMetrics(file); - if (metrics.splitLineCount === undefined) { - metrics.splitLineCount = buildSplitDiffRows(file).length; - } - return metrics.splitLineCount; -} +const EMPTY_NUMBER_LIST: number[] = []; function computeEmptyMessage( hideWhitespace: boolean, @@ -1998,6 +2061,9 @@ interface SharedDiffViewProps { }; } +type WorkingTreeDiffMode = Extract; +const EMPTY_WORKING_TREE_DIFF_MODE: Partial = {}; + export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffViewProps) { const isCompact = useIsCompactFormFactor(); const { layout, wrapLines, codeFontSize, monoFontFamily } = displayPreferences; @@ -2011,7 +2077,22 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ...(trimmedMonoFontFamily ? { fontFamily: trimmedMonoFontFamily } : null), }; }, [codeFontSize, diffBodyLineHeight, monoFontFamily]); - const viewMode = mode.kind === "working_tree" ? mode.viewMode : "flat"; + const workingTreeMode = mode.kind === "working_tree" ? mode : EMPTY_WORKING_TREE_DIFF_MODE; + const { + viewMode = "flat", + collapsedFolders: collapsedFoldersArray = EMPTY_PATH_LIST, + onFilePress, + onOpenFile, + onAddToChat, + workspaceFileDragScope, + onCopyPath, + onCopyRelativePath, + onReveal, + revealTargetName, + onDownload, + onDuplicate, + onRevert, + } = workingTreeMode; const expandedPathsArray = useMemo(() => { if (mode.kind === "working_tree") { return mode.expandedPaths; @@ -2022,26 +2103,12 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi return files.map((file) => file.path); }, [files, mode]); const expandedPaths = useMemo(() => new Set(expandedPathsArray), [expandedPathsArray]); - const collapsedFoldersArray = - mode.kind === "working_tree" ? mode.collapsedFolders : EMPTY_PATH_LIST; const collapsedFolders = useMemo(() => new Set(collapsedFoldersArray), [collapsedFoldersArray]); const stickyHeaders = mode.kind !== "commit"; const interactive = mode.kind !== "commit"; const reviewActions = mode.kind === "commit" ? undefined : mode.reviewActions; - const onFilePress = mode.kind === "working_tree" ? mode.onFilePress : undefined; const focusPath = mode.kind === "working_tab" ? mode.focusPath : undefined; const focusRequestId = mode.kind === "working_tab" ? mode.focusRequestId : undefined; - const onOpenFile = mode.kind === "working_tree" ? mode.onOpenFile : undefined; - const onAddToChat = mode.kind === "working_tree" ? mode.onAddToChat : undefined; - const workspaceFileDragScope = - mode.kind === "working_tree" ? mode.workspaceFileDragScope : undefined; - const onCopyPath = mode.kind === "working_tree" ? mode.onCopyPath : undefined; - const onCopyRelativePath = mode.kind === "working_tree" ? mode.onCopyRelativePath : undefined; - const onReveal = mode.kind === "working_tree" ? mode.onReveal : undefined; - const revealTargetName = mode.kind === "working_tree" ? mode.revealTargetName : undefined; - const onDownload = mode.kind === "working_tree" ? mode.onDownload : undefined; - const onDuplicate = mode.kind === "working_tree" ? mode.onDuplicate : undefined; - const onRevert = mode.kind === "working_tree" ? mode.onRevert : undefined; // Keep selection independent from expansion so future keyboard actions (such as R to rename) // can target the current VCS file or folder without changing its open state. const [selectedPath, setSelectedPath] = useState(null); @@ -2062,19 +2129,24 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi () => new Set(Array.from(collapsedFolders).filter((path) => allFolderPathSet.has(path))), [allFolderPathSet, collapsedFolders], ); - const diffListRef = useRef>(null); + const diffListRef = useRef>(null); const scrollbar = useOverlayFlatListScrollbar(diffListRef, { enabled: !isCompact }); const { onLayout: updateScrollbarLayout, onScroll: updateScrollbarOffset } = scrollbar; const consumedFocusRequestRef = useRef(null); const pendingFocusRequestRef = useRef(null); const diffListScrollOffsetRef = useRef(0); const diffListViewportHeightRef = useRef(0); + const pendingMeasuredHeightCorrectionRef = useRef(0); + const [diffListWidth, setDiffListWidth] = useState(800); const headerHeightByPathRef = useRef>({}); - const bodyHeightByKeyRef = useRef>({}); + const measuredGroupHeightsRef = useRef(new Map()); + const rowCacheRef = useRef(createDiffRowCache()); const folderRowHeightRef = useRef(0); const defaultHeaderHeightRef = useRef(44); const [heightVersion, setHeightVersion] = useState(0); + const [geometryVersion, setGeometryVersion] = useState(0); const heightVersionFrameRef = useRef(null); + const geometryVersionFrameRef = useRef(null); const scheduleHeightVersionUpdate = useCallback(() => { if (heightVersionFrameRef.current !== null) { return; @@ -2084,82 +2156,106 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi setHeightVersion((version) => version + 1); }); }, []); + const scheduleGeometryVersionUpdate = useCallback(() => { + if (geometryVersionFrameRef.current !== null) { + return; + } + geometryVersionFrameRef.current = requestAnimationFrame(() => { + geometryVersionFrameRef.current = null; + setGeometryVersion((version) => version + 1); + }); + }, []); useEffect( () => () => { if (heightVersionFrameRef.current !== null) { cancelAnimationFrame(heightVersionFrameRef.current); } + if (geometryVersionFrameRef.current !== null) { + cancelAnimationFrame(geometryVersionFrameRef.current); + } }, [], ); const diffBodyChromeHeight = BORDER_WIDTH[1] * 2; const statusBodyHeightEstimate = diffBodyChromeHeight + SPACING[4] * 2 + diffBodyLineHeight; - const { flatItems, stickyHeaderIndices } = useMemo(() => { - const { items, stickyHeaderIndices: stickyIndices } = buildDiffFlatItems({ + const reviewHeightFor = useCallback( + (target: ReviewableDiffTarget | null | undefined): number => + getInlineReviewThreadState({ reviewTarget: target, reviewActions })?.height ?? 0, + [reviewActions], + ); + const geometryKey = `${layout}:${wrapLines ? "wrap" : "scroll"}:${typographyKey}:${Math.round(diffListWidth)}`; + const rowIndex = useMemo(() => { + void heightVersion; + return buildDiffRowIndex({ files, viewMode, tree: compressedTree, collapsedFolders: effectiveCollapsedFolders, expandedPaths, + layout, + wrapLines, + lineHeight: diffBodyLineHeight, + viewportWidth: diffListWidth, + codeFontSize, + headerHeightFor: (file) => + headerHeightByPathRef.current[file.path] ?? defaultHeaderHeightRef.current, + folderHeight: folderRowHeightRef.current || defaultHeaderHeightRef.current, + statusHeight: statusBodyHeightEstimate, + measuredGroupHeights: measuredGroupHeightsRef.current, + geometryKey, + reviewHeightFor, + rowCache: rowCacheRef.current, }); - return { - flatItems: items, - stickyHeaderIndices: stickyHeaders ? stickyIndices : [], - }; - }, [compressedTree, effectiveCollapsedFolders, expandedPaths, files, stickyHeaders, viewMode]); - - const getBodyHeightKey = useCallback( - (file: ParsedDiffFile): string => { - if (file.status === "too_large" || file.status === "binary") { - return `${layout}:${wrapLines ? "wrap" : "scroll"}:${typographyKey}:${file.path}:${file.status}`; - } - - const metrics = getDiffFileMetrics(file); - return [ - layout, - wrapLines ? "wrap" : "scroll", - typographyKey, - file.path, - file.status ?? "ok", - file.additions, - file.deletions, - file.hunks.length, - metrics.unifiedLineCount, - metrics.contentLength, - ].join(":"); - }, - [layout, typographyKey, wrapLines], + }, [ + codeFontSize, + compressedTree, + diffBodyLineHeight, + diffListWidth, + effectiveCollapsedFolders, + expandedPaths, + files, + geometryKey, + heightVersion, + layout, + reviewHeightFor, + statusBodyHeightEstimate, + viewMode, + wrapLines, + ]); + const flatItems = rowIndex.items; + const horizontalContentColumns = useMemo( + () => + flatItems.reduce( + (maximum, item) => + item.type === "unified_group" || item.type === "split_group" + ? Math.max(maximum, item.contentColumns) + : maximum, + 0, + ), + [flatItems], ); - - const estimateBodyHeight = useCallback( - (file: ParsedDiffFile): number => { - if (file.status === "too_large" || file.status === "binary") { - return statusBodyHeightEstimate; - } - - const lineCount = - layout === "split" - ? getSplitDiffLineCount(file) - : getDiffFileMetrics(file).unifiedLineCount; - return diffBodyChromeHeight + lineCount * diffBodyLineHeight; - }, - [diffBodyChromeHeight, diffBodyLineHeight, layout, statusBodyHeightEstimate], + const maximumGutterWidth = useMemo( + () => + files.reduce( + (maximum, file) => + Math.max(maximum, lineNumberGutterWidth(maxLineNumber(file), codeFontSize)), + 0, + ), + [codeFontSize, files], ); - - const getFlatItemHeight = useCallback( - (item: DiffFlatItem): number => { - if (item.type === "folder") { - return folderRowHeightRef.current || defaultHeaderHeightRef.current; - } - if (item.type === "header") { - return headerHeightByPathRef.current[item.file.path] ?? defaultHeaderHeightRef.current; - } - const bodyHeightKey = getBodyHeightKey(item.file); - return bodyHeightByKeyRef.current[bodyHeightKey] ?? estimateBodyHeight(item.file); - }, - [estimateBodyHeight, getBodyHeightKey], + const horizontalCodeWidth = Math.ceil( + horizontalContentColumns * codeFontSize * 0.75 + SPACING[4], ); + const horizontalCodeViewportWidth = Math.max( + 0, + (layout === "split" ? diffListWidth / 2 : diffListWidth) - maximumGutterWidth, + ); + const horizontalSurfaceWidth = + diffListWidth + Math.max(0, horizontalCodeWidth - horizontalCodeViewportWidth); + const rowIndexRef = useRef(rowIndex); + rowIndexRef.current = rowIndex; + const stickyHeaderIndices = stickyHeaders ? rowIndex.stickyHeaderIndices : EMPTY_NUMBER_LIST; const handleFolderRowHeightChange = useCallback( (height: number) => { @@ -2195,28 +2291,48 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi [scheduleHeightVersionUpdate], ); - const handleBodyHeightChange = useCallback( - (file: ParsedDiffFile, height: number) => { + const handleGroupHeightChange = useCallback( + (key: string, height: number) => { if (!Number.isFinite(height) || height < 0) { return; } - const heightKey = getBodyHeightKey(file); - const previousHeight = bodyHeightByKeyRef.current[heightKey]; + const previousHeight = measuredGroupHeightsRef.current.get(key); if ( previousHeight !== undefined && Math.abs(previousHeight - height) <= DIFF_HEIGHT_CHANGE_EPSILON ) { return; } - bodyHeightByKeyRef.current[heightKey] = height; - scheduleHeightVersionUpdate(); + const currentIndex = rowIndexRef.current; + const update = currentIndex.updateHeight(key, height); + if (!update) return; + if ( + currentIndex.offsetAt(update.index) + update.previousHeight <= + diffListScrollOffsetRef.current + ) { + pendingMeasuredHeightCorrectionRef.current += update.delta; + } + measuredGroupHeightsRef.current.set(key, height); + scheduleGeometryVersionUpdate(); }, - [getBodyHeightKey, scheduleHeightVersionUpdate], + [scheduleGeometryVersionUpdate], ); + useEffect(() => { + const correction = pendingMeasuredHeightCorrectionRef.current; + if (correction === 0) { + return; + } + pendingMeasuredHeightCorrectionRef.current = 0; + const offset = Math.max(0, diffListScrollOffsetRef.current + correction); + diffListScrollOffsetRef.current = offset; + diffListRef.current?.scrollToOffset({ offset, animated: false }); + }, [geometryVersion]); + const handleDiffListScroll = useCallback( (event: NativeSyntheticEvent) => { - diffListScrollOffsetRef.current = event.nativeEvent.contentOffset.y; + const offset = event.nativeEvent.contentOffset.y; + diffListScrollOffsetRef.current = offset; updateScrollbarOffset(event); }, [updateScrollbarOffset], @@ -2228,6 +2344,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi if (!Number.isFinite(height) || height <= 0) { return; } + setDiffListWidth(event.nativeEvent.layout.width); diffListViewportHeightRef.current = height; updateScrollbarLayout(event); }, @@ -2235,20 +2352,19 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ); const computeItemOffset = useCallback( - (predicate: (item: DiffFlatItem) => boolean): number | null => { + (predicate: (item: DiffListItem) => boolean): number | null => { const index = flatItems.findIndex(predicate); if (index < 0) { return null; } - return sumHeightsBefore(flatItems, index, getFlatItemHeight); + return rowIndex.offsetAt(index); }, - [flatItems, getFlatItemHeight], + [flatItems, rowIndex], ); const computeHeaderOffset = useCallback( - (path: string): number => - computeItemOffset((item) => item.type === "header" && item.file.path === path) ?? 0, - [computeItemOffset], + (path: string): number => rowIndex.offsetOfFile(path), + [rowIndex], ); useEffect(() => { @@ -2388,7 +2504,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ); const renderFlatItem = useCallback( - ({ item }: { item: DiffFlatItem }) => { + ({ item }: { item: DiffListItem }) => { if (item.type === "folder") { return ( ); } + if (item.type === "status") { + return ; + } return ( - ); }, [ codeFontSize, - handleBodyHeightChange, + diffListWidth, + handleGroupHeightChange, handleFolderRowHeightChange, handleHeaderHeightChange, handleCollapseFolder, handleSelectPath, handleToggleExpanded, handleToggleFolder, - layout, reviewActions, workspaceFileDragScope, textMetricsStyle, @@ -2482,20 +2600,26 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ], ); - const flatKeyExtractor = useCallback( - (item: DiffFlatItem) => - item.type === "folder" ? `folder-${item.dirPath}` : `${item.type}-${item.file.path}`, - [], - ); + const flatKeyExtractor = useCallback((item: DiffListItem) => { + if (item.type === "folder") { + return `folder-${item.dirPath}`; + } + if (item.type === "unified_group" || item.type === "split_group") { + return item.key; + } + return `${item.type}-${item.file.path}`; + }, []); - const getFlatItemLayout = useCallback( + const getFlatItemLayout = useCallback( (_data, index) => { - const offset = sumHeightsBefore(flatItems, index, getFlatItemHeight); - const item = flatItems[index]; - const length = item ? getFlatItemHeight(item) : 0; - return { length, offset, index }; + void geometryVersion; + return { + length: rowIndex.heightAt(index), + offset: rowIndex.offsetAt(index), + index, + }; }, - [flatItems, getFlatItemHeight], + [geometryVersion, rowIndex], ); const flatExtraData = useMemo( @@ -2505,6 +2629,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi layout, typographyKey, heightVersion, + geometryVersion, viewMode, wrapLines, reviewActions, @@ -2514,6 +2639,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi expandedPathsArray, collapsedFoldersArray, heightVersion, + geometryVersion, layout, reviewActions, typographyKey, @@ -2523,29 +2649,45 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ], ); + const diffList = ( + + ); + return ( - + {wrapLines ? ( + diffList + ) : ( + + {diffList} + + )} {scrollbar.overlay} ); @@ -3502,6 +3644,9 @@ const styles = StyleSheet.create((theme) => ({ borderTopColor: theme.colors.border, backgroundColor: theme.colors.surface1, }, + diffGroupContent: { + backgroundColor: theme.colors.surface1, + }, diffContentRow: { flexDirection: "row", alignItems: "stretch", @@ -3548,6 +3693,7 @@ const styles = StyleSheet.create((theme) => ({ }, splitColumnScroll: { flex: 1, + minWidth: 0, }, splitHeaderRow: { backgroundColor: theme.colors.surface2, diff --git a/packages/app/src/git/diff-rows/model.test.ts b/packages/app/src/git/diff-rows/model.test.ts new file mode 100644 index 000000000..bd74ffe24 --- /dev/null +++ b/packages/app/src/git/diff-rows/model.test.ts @@ -0,0 +1,286 @@ +import { describe, expect, it } from "vitest"; +import type { ParsedDiffFile } from "@/git/use-diff-query"; +import { + buildDiffRowIndex, + createDiffRowCache, + DIFF_VIRTUALIZATION_CONFIG, + type DiffRowCache, +} from "./model"; + +function createFile(path: string, lineCount: number): ParsedDiffFile { + return { + path, + isNew: false, + isDeleted: false, + additions: lineCount, + deletions: 0, + hunks: [ + { + oldStart: 1, + oldCount: 0, + newStart: 1, + newCount: lineCount, + lines: [ + { type: "header", content: `@@ -1,0 +1,${lineCount} @@` }, + ...Array.from({ length: lineCount }, (_, index) => ({ + type: "add" as const, + content: `line ${index + 1}`, + })), + ], + }, + ], + }; +} + +function buildIndex(files: ParsedDiffFile[], expandedPaths: string[]) { + return buildDiffRowIndex({ + files, + viewMode: "flat", + collapsedFolders: new Set(), + expandedPaths: new Set(expandedPaths), + layout: "unified", + wrapLines: false, + lineHeight: 18, + viewportWidth: 400, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: "test", + reviewHeightFor: () => 0, + }); +} + +function containsReviewedLine(group: { + lines: Array<{ reviewTarget?: { lineNumber?: number } | null }>; +}): boolean { + return group.lines.some((line) => line.reviewTarget?.lineNumber === 10); +} + +function summarizeTree(items: ReturnType["items"]): string[] { + return items.map((item) => { + if (item.type === "folder") { + return `${" ".repeat(item.depth)}[${item.displayName}]${item.collapsed ? " (collapsed)" : ""}`; + } + const base = item.file.path.split("/").pop(); + const isGroup = item.type === "unified_group" || item.type === "split_group"; + return `${" ".repeat(item.depth)}${isGroup ? "group:" : ""}${base}`; + }); +} + +describe("buildDiffRowIndex", () => { + it("turns a large expanded file into bounded native-list cells", () => { + const index = buildIndex([createFile("large.ts", 1_200)], ["large.ts"]); + const groups = index.items.filter((item) => item.type === "unified_group"); + + expect(groups.length).toBeGreaterThan(1); + expect(Math.max(...groups.map((group) => group.lines.length))).toBeLessThanOrEqual( + DIFF_VIRTUALIZATION_CONFIG.groupRowLimit, + ); + expect(groups.reduce((count, group) => count + group.lines.length, 0)).toBe(1_201); + expect(index.stickyHeaderIndices).toEqual([0]); + }); + + it("counts wide glyphs and tabs conservatively for horizontal scroll extent", () => { + for (const [path, content] of [ + ["cjk.ts", "界".repeat(40)], + ["emoji.ts", "😀".repeat(40)], + ["tabs.ts", "\t".repeat(10)], + ]) { + const file = createFile(path, 1); + file.hunks[0].lines[1].content = content; + + const index = buildIndex([file], [file.path]); + const group = index.items.find((item) => item.type === "unified_group"); + + expect(group?.contentColumns).toBe(80); + } + }); + + it("precomputes exact offsets and preserves total file height", () => { + const index = buildIndex( + [createFile("first.ts", 65), createFile("second.ts", 1)], + ["first.ts"], + ); + const firstGroups = index.items.filter( + (item) => item.type === "unified_group" && item.file.path === "first.ts", + ); + const secondHeaderIndex = index.items.findIndex( + (item) => item.type === "header" && item.file.path === "second.ts", + ); + const expectedBodyHeight = 66 * 18 + 2; + + expect(firstGroups).toHaveLength(5); + expect(index.offsetAt(secondHeaderIndex)).toBe(44 + expectedBodyHeight); + expect(index.offsetOfFile("second.ts")).toBe(44 + expectedBodyHeight); + expect(index.totalHeight).toBe(44 + expectedBodyHeight + 44); + }); + + it("keeps a review row whole and shifts every later offset by its height", () => { + const file = createFile("review.ts", 40); + const baseline = buildIndex([file], [file.path]); + const reviewed = buildDiffRowIndex({ + files: [file], + viewMode: "flat", + collapsedFolders: new Set(), + expandedPaths: new Set([file.path]), + layout: "unified", + wrapLines: false, + lineHeight: 18, + viewportWidth: 400, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: "test", + reviewHeightFor: (target) => (target?.lineNumber === 10 ? 132 : 0), + }); + + expect(reviewed.totalHeight - baseline.totalHeight).toBe(132); + const reviewGroup = reviewed.items + .filter((item) => item.type === "unified_group") + .find(containsReviewedLine); + expect(reviewGroup).toBeDefined(); + expect(reviewed.heightAt(reviewed.items.indexOf(reviewGroup!))).toBeGreaterThanOrEqual(150); + }); + + it("preserves compressed tree order, indentation, and collapse state", () => { + const files = [createFile("src/app/a.ts", 1), createFile("src/app/nested/b.ts", 1)]; + const index = buildDiffRowIndex({ + files, + viewMode: "tree", + collapsedFolders: new Set(["src/app/nested"]), + expandedPaths: new Set(["src/app/a.ts"]), + layout: "unified", + wrapLines: false, + lineHeight: 18, + viewportWidth: 400, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: "tree", + reviewHeightFor: () => 0, + }); + + expect(summarizeTree(index.items)).toEqual([ + "[src/app]", + " [nested] (collapsed)", + " a.ts", + " group:a.ts", + ]); + expect(index.stickyHeaderIndices).toEqual([2]); + }); + + it("keeps split groups bounded and sticky indices on headers", () => { + const file = createFile("split.ts", 100); + const index = buildDiffRowIndex({ + files: [file], + viewMode: "flat", + collapsedFolders: new Set(), + expandedPaths: new Set([file.path]), + layout: "split", + wrapLines: false, + lineHeight: 18, + viewportWidth: 800, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: "split", + reviewHeightFor: () => 0, + }); + const groups = index.items.filter((item) => item.type === "split_group"); + + expect(Math.max(...groups.map((group) => group.rows.length))).toBeLessThanOrEqual( + DIFF_VIRTUALIZATION_CONFIG.groupRowLimit, + ); + expect(index.items[index.stickyHeaderIndices[0]].type).toBe("header"); + }); + + it("updates measured group geometry without deriving any file rows again", () => { + const delegate = createDiffRowCache(); + let derivations = 0; + const rowCache: DiffRowCache = { + unifiedFor(file) { + derivations += 1; + return delegate.unifiedFor(file); + }, + splitFor(file) { + derivations += 1; + return delegate.splitFor(file); + }, + }; + const files = [createFile("first.ts", 100), createFile("second.ts", 100)]; + const index = buildDiffRowIndex({ + files, + viewMode: "flat", + collapsedFolders: new Set(), + expandedPaths: new Set(files.map((file) => file.path)), + layout: "unified", + wrapLines: true, + lineHeight: 18, + viewportWidth: 400, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: "wrap", + reviewHeightFor: () => 0, + rowCache, + }); + const group = index.items.find((item) => item.type === "unified_group"); + expect(group).toBeDefined(); + expect(derivations).toBe(2); + + const groupIndex = index.indexOfGroup(group!.key); + const laterOffset = index.offsetAt(groupIndex + 1); + const update = index.updateHeight(group!.key, index.heightAt(groupIndex) + 20); + + expect(update?.delta).toBe(20); + expect(index.offsetAt(groupIndex + 1)).toBe(laterOffset + 20); + expect(derivations).toBe(2); + }); + + it("uses the key's rounded viewport width for wrapped group membership", () => { + const file = createFile("wrapped.ts", 40); + for (const line of file.hunks[0].lines) { + if (line.type !== "header") { + line.content = "x".repeat(88); + } + } + const buildWrapped = (viewportWidth: number) => + buildDiffRowIndex({ + files: [file], + viewMode: "flat", + collapsedFolders: new Set(), + expandedPaths: new Set([file.path]), + layout: "unified", + wrapLines: true, + lineHeight: 18, + viewportWidth, + codeFontSize: 12, + headerHeightFor: () => 44, + folderHeight: 44, + statusHeight: 64, + measuredGroupHeights: new Map(), + geometryKey: `wrap:${Math.round(viewportWidth)}`, + reviewHeightFor: () => 0, + }); + const summarizeGroups = (viewportWidth: number) => + buildWrapped(viewportWidth) + .items.filter((item) => item.type === "unified_group") + .map((group) => ({ + key: group.key, + startRowIndex: group.startRowIndex, + lineCount: group.lines.length, + })); + + expect(summarizeGroups(399.2)).toEqual(summarizeGroups(399.49)); + }); +}); diff --git a/packages/app/src/git/diff-rows/model.ts b/packages/app/src/git/diff-rows/model.ts new file mode 100644 index 000000000..0206c8aa6 --- /dev/null +++ b/packages/app/src/git/diff-rows/model.ts @@ -0,0 +1,422 @@ +import type { ParsedDiffFile } from "@/git/use-diff-query"; +import { + buildDiffTree, + compressSingleChildChains, + flattenDiffTree, + type DiffTreeDirNode, +} from "@/git/diff-tree"; +import { + buildSplitDiffRows, + buildUnifiedDiffLines, + type ReviewableDiffTarget, + type SplitDiffRow, + type UnifiedDiffDisplayLine, +} from "@/utils/diff-layout"; + +// Tune cell size and the outer list window together. On the 1,200-row fixture, the exact +// three-fling repro filled immediately with both 32/720 + window 21 and this lower-cost pair +// on an iPhone 17 Pro simulator. Simulator frame percentiles are unavailable in xctrace. +export const DIFF_VIRTUALIZATION_CONFIG = { + groupRowLimit: 16, + groupHeightLimit: 360, + initialNumToRender: 8, + maxToRenderPerBatch: 8, + updateCellsBatchingPeriod: 16, + windowSize: 10, +} as const; + +interface FolderItem { + type: "folder"; + dirPath: string; + displayName: string; + depth: number; + collapsed: boolean; + additions: number; + deletions: number; +} + +interface HeaderItem { + type: "header"; + file: ParsedDiffFile; + fileIndex: number; + isExpanded: boolean; + depth: number; +} + +interface StatusItem { + type: "status"; + file: ParsedDiffFile; + fileIndex: number; + depth: number; +} + +interface RowGroupBase { + file: ParsedDiffFile; + fileIndex: number; + depth: number; + groupIndex: number; + startRowIndex: number; + isFirst: boolean; + isLast: boolean; + contentColumns: number; + key: string; +} + +export interface UnifiedRowGroupItem extends RowGroupBase { + type: "unified_group"; + lines: UnifiedDiffDisplayLine[]; +} + +export interface SplitRowGroupItem extends RowGroupBase { + type: "split_group"; + rows: SplitDiffRow[]; +} + +export type DiffListItem = + | FolderItem + | HeaderItem + | StatusItem + | UnifiedRowGroupItem + | SplitRowGroupItem; + +export interface DiffRowIndex { + items: DiffListItem[]; + stickyHeaderIndices: number[]; + heightAt: (index: number) => number; + offsetAt: (index: number) => number; + offsetOfFile: (path: string) => number; + indexOfFile: (path: string) => number; + indexOfGroup: (key: string) => number; + updateHeight: ( + key: string, + height: number, + ) => { delta: number; index: number; previousHeight: number } | null; + totalHeight: number; +} + +export interface DiffRowCache { + unifiedFor: (file: ParsedDiffFile) => UnifiedDiffDisplayLine[]; + splitFor: (file: ParsedDiffFile) => SplitDiffRow[]; +} + +export function createDiffRowCache(): DiffRowCache { + const unified = new WeakMap(); + const split = new WeakMap(); + return { + unifiedFor(file) { + const cached = unified.get(file); + if (cached) return cached; + const lines = buildUnifiedDiffLines(file); + unified.set(file, lines); + return lines; + }, + splitFor(file) { + const cached = split.get(file); + if (cached) return cached; + const rows = buildSplitDiffRows(file); + split.set(file, rows); + return rows; + }, + }; +} + +const defaultRowCache = createDiffRowCache(); + +export interface BuildDiffRowIndexInput { + files: ParsedDiffFile[]; + viewMode: "flat" | "tree"; + collapsedFolders: ReadonlySet; + expandedPaths: ReadonlySet; + layout: "unified" | "split"; + wrapLines: boolean; + lineHeight: number; + viewportWidth: number; + codeFontSize: number; + headerHeightFor: (file: ParsedDiffFile) => number; + folderHeight: number; + statusHeight: number; + measuredGroupHeights: ReadonlyMap; + geometryKey: string; + reviewHeightFor: (target: ReviewableDiffTarget | null | undefined) => number; + rowCache?: DiffRowCache; + tree?: DiffTreeDirNode; +} + +interface PendingGroup { + rows: Row[]; + startRowIndex: number; + height: number; +} + +const FULL_WIDTH_CODE_POINT_RANGES = [ + [0x1100, 0x115f], + [0x2329, 0x232a], + [0x2e80, 0x303e], + [0x3040, 0x3247], + [0x3250, 0x4dbf], + [0x4e00, 0xa4c6], + [0xa960, 0xa97c], + [0xac00, 0xd7a3], + [0xf900, 0xfaff], + [0xfe10, 0xfe19], + [0xfe30, 0xfe6b], + [0xff01, 0xff60], + [0xffe0, 0xffe6], + [0x1b000, 0x1b001], + [0x1f200, 0x1f251], + [0x1f300, 0x1faff], + [0x20000, 0x3fffd], +] as const; + +function isFullWidthCodePoint(codePoint: number): boolean { + return FULL_WIDTH_CODE_POINT_RANGES.some( + ([start, end]) => codePoint >= start && codePoint <= end, + ); +} + +function displayColumns(content: string): number { + let columns = 0; + for (const character of content) { + if (character === "\t") { + columns += 8 - (columns % 8); + continue; + } + columns += isFullWidthCodePoint(character.codePointAt(0) ?? 0) ? 2 : 1; + } + return columns; +} + +function estimatedVisualLines(content: string, columns: number): number { + return Math.max(1, Math.ceil(displayColumns(content) / columns)); +} + +function groupRows(rows: Row[], heightFor: (row: Row) => number): PendingGroup[] { + const groups: PendingGroup[] = []; + let current: PendingGroup = { rows: [], startRowIndex: 0, height: 0 }; + + for (const [rowIndex, row] of rows.entries()) { + const rowHeight = heightFor(row); + const reachedRowLimit = current.rows.length >= DIFF_VIRTUALIZATION_CONFIG.groupRowLimit; + const reachedHeightLimit = + current.rows.length > 0 && + current.height + rowHeight > DIFF_VIRTUALIZATION_CONFIG.groupHeightLimit; + if (reachedRowLimit || reachedHeightLimit) { + groups.push(current); + current = { rows: [], startRowIndex: rowIndex, height: 0 }; + } + current.rows.push(row); + current.height += rowHeight; + } + + if (current.rows.length > 0) { + groups.push(current); + } + return groups; +} + +function contentColumnsForUnified(lines: UnifiedDiffDisplayLine[]): number { + return lines.reduce((longest, line) => Math.max(longest, displayColumns(line.line.content)), 0); +} + +function contentColumnsForSplit(rows: SplitDiffRow[]): number { + return rows.reduce((longest, row) => { + if (row.kind === "header") { + return Math.max(longest, displayColumns(row.content)); + } + return Math.max( + longest, + displayColumns(row.left?.content ?? ""), + displayColumns(row.right?.content ?? ""), + ); + }, 0); +} + +function availableTextColumns(input: BuildDiffRowIndexInput): number { + const viewportWidth = Math.round(input.viewportWidth); + const columnWidth = input.layout === "split" ? viewportWidth / 2 : viewportWidth; + const characterWidth = input.codeFontSize * 0.62; + return Math.max(8, Math.floor((columnWidth - 72) / characterWidth)); +} + +export function buildDiffRowIndex(input: BuildDiffRowIndexInput): DiffRowIndex { + const items: DiffListItem[] = []; + const heights: number[] = []; + const stickyHeaderIndices: number[] = []; + const fileIndexByPath = new Map(); + const groupIndexByKey = new Map(); + const availableColumns = availableTextColumns(input); + const rowCache = input.rowCache ?? defaultRowCache; + + function push(item: DiffListItem, height: number): void { + if (item.type === "unified_group" || item.type === "split_group") { + groupIndexByKey.set(item.key, items.length); + } + items.push(item); + heights.push(height); + } + + function pushUnifiedGroups(file: ParsedDiffFile, fileIndex: number, depth: number): void { + const lines = rowCache.unifiedFor(file); + const contentColumns = contentColumnsForUnified(lines); + const groups = groupRows(lines, (line) => { + const visualLines = input.wrapLines + ? estimatedVisualLines(line.line.content, availableColumns) + : 1; + return visualLines * input.lineHeight + input.reviewHeightFor(line.reviewTarget); + }); + + for (const [groupIndex, group] of groups.entries()) { + const isFirst = groupIndex === 0; + const isLast = groupIndex === groups.length - 1; + const key = `${input.geometryKey}:${file.path}:unified:${group.startRowIndex}`; + const estimatedHeight = group.height + (isFirst ? 1 : 0) + (isLast ? 1 : 0); + push( + { + type: "unified_group", + file, + fileIndex, + depth, + groupIndex, + startRowIndex: group.startRowIndex, + isFirst, + isLast, + contentColumns, + key, + lines: group.rows, + }, + input.measuredGroupHeights.get(key) ?? estimatedHeight, + ); + } + } + + function pushSplitGroups(file: ParsedDiffFile, fileIndex: number, depth: number): void { + const rows = rowCache.splitFor(file); + const contentColumns = contentColumnsForSplit(rows); + const groups = groupRows(rows, (row) => { + if (row.kind === "header") { + const visualLines = input.wrapLines + ? estimatedVisualLines(row.content, availableColumns) + : 1; + return visualLines * input.lineHeight; + } + const visualLines = input.wrapLines + ? Math.max( + estimatedVisualLines(row.left?.content ?? "", availableColumns), + estimatedVisualLines(row.right?.content ?? "", availableColumns), + ) + : 1; + const reviewHeight = Math.max( + input.reviewHeightFor(row.left?.reviewTarget), + input.reviewHeightFor(row.right?.reviewTarget), + ); + return visualLines * input.lineHeight + reviewHeight; + }); + + for (const [groupIndex, group] of groups.entries()) { + const isFirst = groupIndex === 0; + const isLast = groupIndex === groups.length - 1; + const key = `${input.geometryKey}:${file.path}:split:${group.startRowIndex}`; + const estimatedHeight = group.height + (isFirst ? 1 : 0) + (isLast ? 1 : 0); + push( + { + type: "split_group", + file, + fileIndex, + depth, + groupIndex, + startRowIndex: group.startRowIndex, + isFirst, + isLast, + contentColumns, + key, + rows: group.rows, + }, + input.measuredGroupHeights.get(key) ?? estimatedHeight, + ); + } + } + + function pushFile(file: ParsedDiffFile, fileIndex: number, depth: number): void { + const isExpanded = input.expandedPaths.has(file.path); + const headerIndex = items.length; + fileIndexByPath.set(file.path, headerIndex); + push({ type: "header", file, fileIndex, isExpanded, depth }, input.headerHeightFor(file)); + if (!isExpanded) { + return; + } + stickyHeaderIndices.push(headerIndex); + if (file.status === "binary" || file.status === "too_large") { + push({ type: "status", file, fileIndex, depth }, input.statusHeight); + return; + } + if (input.layout === "split") { + pushSplitGroups(file, fileIndex, depth); + return; + } + pushUnifiedGroups(file, fileIndex, depth); + } + + if (input.viewMode === "flat") { + for (const [fileIndex, file] of input.files.entries()) { + pushFile(file, fileIndex, 0); + } + } else { + const indexByPath = new Map(input.files.map((file, index) => [file.path, index])); + const tree = input.tree ?? compressSingleChildChains(buildDiffTree(input.files)); + for (const row of flattenDiffTree(tree, input.collapsedFolders)) { + if (row.kind === "folder") { + push( + { + type: "folder", + dirPath: row.dirPath, + displayName: row.displayName, + depth: row.depth, + collapsed: input.collapsedFolders.has(row.dirPath), + additions: row.additions, + deletions: row.deletions, + }, + input.folderHeight, + ); + continue; + } + const fileIndex = indexByPath.get(row.file.path); + if (fileIndex !== undefined) { + pushFile(row.file, fileIndex, row.depth); + } + } + } + + const offsets = Array.from({ length: items.length + 1 }, () => 0); + for (let index = 0; index < items.length; index += 1) { + offsets[index + 1] = offsets[index] + heights[index]; + } + + return { + items, + stickyHeaderIndices, + heightAt: (index) => heights[index] ?? 0, + offsetAt: (index) => offsets[Math.max(0, Math.min(index, items.length))], + offsetOfFile: (path) => offsets[fileIndexByPath.get(path) ?? 0], + indexOfFile: (path) => fileIndexByPath.get(path) ?? -1, + indexOfGroup: (key) => groupIndexByKey.get(key) ?? -1, + updateHeight: (key, height) => { + const index = groupIndexByKey.get(key); + if (index === undefined || !Number.isFinite(height) || height < 0) { + return null; + } + const previousHeight = heights[index]; + const delta = height - previousHeight; + if (delta === 0) { + return { delta, index, previousHeight }; + } + heights[index] = height; + for (let offsetIndex = index + 1; offsetIndex < offsets.length; offsetIndex += 1) { + offsets[offsetIndex] += delta; + } + return { delta, index, previousHeight }; + }, + get totalHeight() { + return offsets[items.length]; + }, + }; +} diff --git a/packages/app/src/git/diff-rows/virtualized-list-range.test.ts b/packages/app/src/git/diff-rows/virtualized-list-range.test.ts new file mode 100644 index 000000000..6392b4912 --- /dev/null +++ b/packages/app/src/git/diff-rows/virtualized-list-range.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { boundRetainedRangeOnDataGrowth } from "@react-native/virtualized-lists/Lists/boundRetainedRangeOnDataGrowth"; + +describe("bounded VirtualizedList data growth", () => { + it("limits the carried range when middle insertion grows the list", () => { + expect(boundRetainedRangeOnDataGrowth({ first: 0, last: 39 }, 40, 115, 16)).toEqual({ + first: 0, + last: 15, + }); + }); + + it("bounds from the current scrolled range instead of jumping to the start", () => { + expect(boundRetainedRangeOnDataGrowth({ first: 84, last: 123 }, 160, 235, 16)).toEqual({ + first: 84, + last: 99, + }); + }); + + it("does not alter the range when data shrinks or the carried range is already bounded", () => { + const shortRange = { first: 12, last: 18 }; + expect(boundRetainedRangeOnDataGrowth(shortRange, 40, 32, 16)).toBe(shortRange); + expect(boundRetainedRangeOnDataGrowth(shortRange, 40, 115, 16)).toEqual(shortRange); + }); +}); diff --git a/packages/app/src/review/surface.tsx b/packages/app/src/review/surface.tsx index fd79290b5..7e80fd86c 100644 --- a/packages/app/src/review/surface.tsx +++ b/packages/app/src/review/surface.tsx @@ -260,6 +260,7 @@ export function InlineReviewGutterCell({ onStartComment, style, actionTestID, + testID, }: { children: ReactNode; reviewTarget: ReviewableDiffTarget | null | undefined; @@ -270,6 +271,7 @@ export function InlineReviewGutterCell({ onStartComment: (target: ReviewableDiffTarget) => void; style?: StyleProp; actionTestID?: string; + testID?: string; }) { const { t } = useTranslation(); const canComment = Boolean(reviewTarget); @@ -309,7 +311,6 @@ export function InlineReviewGutterCell({ } }, [isInteractionActive]); - const pressableStyle = useCallback((): StyleProp => style, [style]); const lineHeightStyle = useMemo>( () => lineHeight !== undefined @@ -318,13 +319,15 @@ export function InlineReviewGutterCell({ [lineHeight], ); - const labelStyle = useMemo>( - () => [styles.gutterLabel, lineHeightStyle, hasComments && styles.gutterLabelActive], - [hasComments, lineHeightStyle], - ); - const innerStyle = useMemo>( - () => [styles.gutterInner, lineHeightStyle], - [lineHeightStyle], + const pressableStyle = useCallback( + (): StyleProp => [ + style, + styles.gutterInner, + styles.gutterLabel, + lineHeightStyle, + hasComments && styles.gutterLabelActive, + ], + [hasComments, lineHeightStyle, style], ); const actionIconStyle = useMemo>( () => [ @@ -346,17 +349,14 @@ export function InlineReviewGutterCell({ onPressIn={handlePressIn} onPressOut={handlePressOut} style={pressableStyle} + testID={testID} > - - - {children} - {showAction ? ( - - - - ) : null} + {children} + {showAction ? ( + + - + ) : null} ); } diff --git a/packages/app/src/types/virtualized-list-internals.d.ts b/packages/app/src/types/virtualized-list-internals.d.ts new file mode 100644 index 000000000..ae357034c --- /dev/null +++ b/packages/app/src/types/virtualized-list-internals.d.ts @@ -0,0 +1,8 @@ +declare module "@react-native/virtualized-lists/Lists/boundRetainedRangeOnDataGrowth" { + export function boundRetainedRangeOnDataGrowth( + cellsAroundViewport: { first: number; last: number }, + previousItemCount: number, + itemCount: number, + retainedCellLimit: number, + ): { first: number; last: number }; +} diff --git a/packages/app/src/utils/diff-rendering.test.ts b/packages/app/src/utils/diff-rendering.test.ts index 4d6e35e89..384bd2291 100644 --- a/packages/app/src/utils/diff-rendering.test.ts +++ b/packages/app/src/utils/diff-rendering.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { + compactHighlightTokens, formatDiffContentText, formatDiffGutterText, hasVisibleDiffTokens, @@ -28,4 +29,23 @@ describe("diff-rendering", () => { expect(hasVisibleDiffTokens([{ text: "" }])).toBe(false); expect(hasVisibleDiffTokens([{ text: "const value = 1;" }])).toBe(true); }); + + it("compacts adjacent syntax tokens without changing their text or styles", () => { + expect( + compactHighlightTokens([ + { text: "const", style: "keyword" }, + { text: " ", style: null }, + { text: "value", style: null }, + { text: " ", style: null }, + { text: "=", style: "operator" }, + { text: "", style: "operator" }, + { text: " 1", style: "number" }, + ]), + ).toEqual([ + { text: "const", style: "keyword" }, + { text: " value ", style: null }, + { text: "=", style: "operator" }, + { text: " 1", style: "number" }, + ]); + }); }); diff --git a/packages/app/src/utils/diff-rendering.ts b/packages/app/src/utils/diff-rendering.ts index a189c56b6..c3cd9f8bb 100644 --- a/packages/app/src/utils/diff-rendering.ts +++ b/packages/app/src/utils/diff-rendering.ts @@ -2,6 +2,28 @@ interface HighlightLikeToken { text: string; } +interface StyledHighlightToken extends HighlightLikeToken { + style: string | null; +} + +export function compactHighlightTokens( + tokens: readonly StyledHighlightToken[], +): StyledHighlightToken[] { + const compacted: StyledHighlightToken[] = []; + for (const token of tokens) { + if (token.text.length === 0) { + continue; + } + const previous = compacted.at(-1); + if (previous?.style === token.style) { + previous.text += token.text; + continue; + } + compacted.push({ text: token.text, style: token.style }); + } + return compacted; +} + // Preserve row height when a gutter or diff cell is intentionally blank. // Non-breaking space because the gutter uses numberOfLines={1}, which // collapses a plain ASCII space to zero height on web. diff --git a/patches/@react-native+virtualized-lists+0.81.5.patch b/patches/@react-native+virtualized-lists+0.81.5.patch new file mode 100644 index 000000000..0ab5e8abc --- /dev/null +++ b/patches/@react-native+virtualized-lists+0.81.5.patch @@ -0,0 +1,105 @@ +diff --git a/node_modules/@react-native/virtualized-lists/Lists/VirtualizedList.js b/node_modules/@react-native/virtualized-lists/Lists/VirtualizedList.js +index c92ac9d..6bdc84f 100644 +--- a/node_modules/@react-native/virtualized-lists/Lists/VirtualizedList.js ++++ b/node_modules/@react-native/virtualized-lists/Lists/VirtualizedList.js +@@ -47,6 +47,7 @@ import { + windowSizeOrDefault, + } from './VirtualizedListProps'; ++import {boundRetainedRangeOnDataGrowth} from './boundRetainedRangeOnDataGrowth'; + import { + computeWindowedRenderLimits, + keyExtractor as defaultKeyExtractor, + } from './VirtualizeUtils'; +@@ -706,7 +706,8 @@ class VirtualizedList extends StateSafePureComponent< + // first and last could be stale (e.g. if a new, shorter items props is passed in), so we make + // sure we're rendering a reasonable range here. + const itemCount = newProps.getItemCount(newProps.data); +- if (itemCount === prevState.renderMask.numCells()) { ++ const previousItemCount = prevState.renderMask.numCells(); ++ if (itemCount === previousItemCount) { + return prevState; + } + +@@ -741,7 +742,7 @@ class VirtualizedList extends StateSafePureComponent< + } + } + +- const constrainedCells = VirtualizedList._constrainToItemCount( ++ let constrainedCells = VirtualizedList._constrainToItemCount( + maintainVisibleContentPositionAdjustment != null + ? { + first: +@@ -756,6 +758,20 @@ class VirtualizedList extends StateSafePureComponent< + newProps, + ); + ++ if ( ++ newProps.unstable_boundedRangeOnDataChange === true && ++ itemCount > previousItemCount ++ ) { ++ const retainedCellLimit = ++ maxToRenderPerBatchOrDefault(newProps.maxToRenderPerBatch) * 2; ++ constrainedCells = boundRetainedRangeOnDataGrowth( ++ constrainedCells, ++ previousItemCount, ++ itemCount, ++ retainedCellLimit, ++ ); ++ } ++ + return { + cellsAroundViewport: constrainedCells, + renderMask: VirtualizedList._createRenderMask(newProps, constrainedCells), +diff --git a/node_modules/@react-native/virtualized-lists/Lists/boundRetainedRangeOnDataGrowth.js b/node_modules/@react-native/virtualized-lists/Lists/boundRetainedRangeOnDataGrowth.js +new file mode 100644 +index 0000000..1e5b1f5 +--- /dev/null ++++ b/node_modules/@react-native/virtualized-lists/Lists/boundRetainedRangeOnDataGrowth.js +@@ -0,0 +1,28 @@ ++/** ++ * Copyright (c) Meta Platforms, Inc. and affiliates. ++ * ++ * This source code is licensed under the MIT license found in the ++ * LICENSE file in the root directory of this source tree. ++ * ++ * @format ++ */ ++ ++'use strict'; ++ ++export function boundRetainedRangeOnDataGrowth( ++ cellsAroundViewport, ++ previousItemCount, ++ itemCount, ++ retainedCellLimit, ++) { ++ if (itemCount <= previousItemCount) { ++ return cellsAroundViewport; ++ } ++ return { ++ first: cellsAroundViewport.first, ++ last: Math.min( ++ cellsAroundViewport.last, ++ cellsAroundViewport.first + retainedCellLimit - 1, ++ ), ++ }; ++} +diff --git a/node_modules/@react-native/virtualized-lists/Lists/VirtualizedListProps.js b/node_modules/@react-native/virtualized-lists/Lists/VirtualizedListProps.js +index 9d6d49e..54324a8 100644 +--- a/node_modules/@react-native/virtualized-lists/Lists/VirtualizedListProps.js ++++ b/node_modules/@react-native/virtualized-lists/Lists/VirtualizedListProps.js +@@ -264,8 +264,14 @@ type OptionalVirtualizedListProps = { + * Amount of time between low-pri item render batches, e.g. for rendering items quite a ways off + * screen. Similar fill rate/responsiveness tradeoff as `maxToRenderPerBatch`. + */ + updateCellsBatchingPeriod?: ?number, ++ ++ /** ++ * Limits the synchronous render range retained when data grows. This is useful for lists that ++ * insert many bounded cells in the middle of an already-rendered range. ++ */ ++ unstable_boundedRangeOnDataChange?: ?boolean, + /** + * See `ViewabilityHelper` for flow type and further documentation. + */ + viewabilityConfig?: ViewabilityConfig, diff --git a/scripts/postinstall-patches.mjs b/scripts/postinstall-patches.mjs index 781294b6d..465a78fc4 100644 --- a/scripts/postinstall-patches.mjs +++ b/scripts/postinstall-patches.mjs @@ -8,6 +8,10 @@ import { join, relative } from "node:path"; // workspace root live in their workspace's own node_modules, and patch-package resolves // the patch's node_modules/... paths relative to its working directory. const patchedPackages = [ + { + nodeModulesPath: "node_modules/@react-native/virtualized-lists", + patchPrefix: "@react-native+virtualized-lists+", + }, { nodeModulesPath: "node_modules/react-native-markdown-display", patchPrefix: "react-native-markdown-display+",