diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index c3b68b145..98f69808b 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 Locator, type Page } from "@playwright/test"; +import { 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,10 +63,6 @@ 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 { @@ -469,12 +465,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(diffGroups(visiblePanel).first()).toBeVisible(); + await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); await expect(page.getByTestId("workspace-file-pane")).toHaveCount(0); await visiblePanel.getByTestId("diff-file-0-toggle").click(); - await expect(diffGroups(visiblePanel)).toHaveCount(0); + await expect(visiblePanel.getByTestId("diff-file-0-body")).toHaveCount(0); await visiblePanel.getByTestId("diff-file-0-toggle").click(); - await expect(diffGroups(visiblePanel).first()).toBeVisible(); + await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); const workingDiffLayoutToggle = visiblePanel.getByTestId("working-diff-toggle-layout"); await expect(workingDiffLayoutToggle).toHaveAccessibleName("Switch to side-by-side diff"); await workingDiffLayoutToggle.click(); @@ -490,12 +486,14 @@ 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+-group-\d+$/)).toHaveCount(0); + await expect(visiblePanel.getByTestId(/^diff-file-\d+-body$/)).toHaveCount(0); await visiblePanel.getByTestId("working-diff-toggle-expand-all").click(); - await expect(diffGroups(visiblePanel).first()).toBeVisible(); + await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); await page.getByTestId("explorer-content-area").getByTestId("diff-file-0-toggle").click(); - await expect(diffGroups(page.getByTestId("explorer-content-area"))).toHaveCount(0); + await expect( + page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), + ).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); @@ -518,9 +516,13 @@ 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(diffGroups(page.getByTestId("explorer-content-area")).first()).toBeVisible(); + await expect( + page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), + ).toBeVisible(); await page.getByTestId("explorer-content-area").getByTestId("diff-file-0-toggle").click(); - await expect(diffGroups(page.getByTestId("explorer-content-area"))).toHaveCount(0); + await expect( + page.getByTestId("explorer-content-area").getByTestId("diff-file-0-body"), + ).toHaveCount(0); }); test("changes diff switches between flat and tree file lists", async ({ page }) => { @@ -583,7 +585,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(diffGroups(page).first()).toBeVisible(); + await expect(page.getByTestId("diff-file-0-body")).toBeVisible(); await page.getByTestId("diff-folder-src-toggle").click(); await expect(page.getByTestId("diff-file-0")).toHaveCount(0); @@ -607,90 +609,6 @@ 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 }) => { @@ -716,12 +634,6 @@ 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 }) => { @@ -770,22 +682,21 @@ async function readVisibleDiffRowGeometry(page: Page): Promise<{ codeLineHeight: number; }[]; }> { - return diffGroups(page).evaluateAll((groups) => { - if (groups.length === 0) { + return page.locator("body").evaluate(({ ownerDocument }) => { + const root = ownerDocument.querySelector('[data-testid="diff-file-0-body"]'); + if (!root) { throw new Error("Expanded diff body is not mounted"); } const readRows = (prefix: string, textPrefix: string) => - 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 }; - }), - ); + 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 }; + }); const gutters = new Map( readRows("diff-gutter-row-", "diff-gutter-text-").map((row) => [row.index, row]), @@ -889,7 +800,7 @@ async function openChangesInVisibleExplorer(page: Page): Promise { } async function expectExpandedMountedTabDiff(page: Page): Promise { - await expect(diffGroups(page).first()).toBeVisible({ timeout: 30_000 }); + await expect(page.getByTestId("diff-file-0-body")).toBeVisible({ timeout: 30_000 }); await expect(page.getByText("function createInitialMountedTabIds")).toBeVisible({ timeout: 30_000, }); @@ -928,9 +839,15 @@ async function returnToWorkspaceChanges(page: Page): Promise { } async function scrollToLowerUnwrappedDiffRows(page: Page): Promise { - 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(); + 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(); } diff --git a/packages/app/e2e/browser/commit-diff-panel.spec.ts b/packages/app/e2e/browser/commit-diff-panel.spec.ts index aa416bb11..1f658fac0 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-group-\d+$/).first()).toBeVisible(); + await expect(panel.getByTestId("diff-file-0-body")).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 d6e3a2a8d..49110c3c7 100644 --- a/packages/app/src/components/diff-scroll.tsx +++ b/packages/app/src/components/diff-scroll.tsx @@ -1,75 +1,58 @@ -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 { useState, useCallback, useEffect, useId, useRef } from "react"; import { - createAnimatedComponent, - useAnimatedScrollHandler, - useSharedValue, - type SharedValue, -} from "react-native-reanimated"; -import { scheduleOnRN } from "react-native-worklets"; + type LayoutChangeEvent, + type NativeSyntheticEvent, + type NativeScrollEvent, + type StyleProp, + type ViewStyle, +} from "react-native"; +import { ScrollView, type ScrollView as ScrollViewType } from "react-native-gesture-handler"; 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 [isAtLeftEdge, setIsAtLeftEdge] = useState(true); - const isAtLeftEdgeValue = useSharedValue(true); + const scrollViewRef = useRef(null); 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 recordScrollOffset = useCallback( - (offsetX: number) => { + 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); 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), @@ -77,7 +60,8 @@ 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 22cca0856..4caa3ac40 100644 --- a/packages/app/src/components/diff-scroll.web.tsx +++ b/packages/app/src/components/diff-scroll.web.tsx @@ -1,23 +1,12 @@ -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"; +import { useCallback } from "react"; +import { ScrollView, type LayoutChangeEvent, type StyleProp, type ViewStyle } from "react-native"; interface DiffScrollProps { children: React.ReactNode; + scrollViewWidth: number; onScrollViewWidthChange: (width: number) => void; style?: StyleProp; contentContainerStyle?: StyleProp; - offset: SharedValue; - testID?: string; } export function DiffScroll({ @@ -25,31 +14,9 @@ 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( - (event: LayoutChangeEvent) => onScrollViewWidthChange(event.nativeEvent.layout.width), + (e: LayoutChangeEvent) => onScrollViewWidthChange(e.nativeEvent.layout.width), [onScrollViewWidthChange], ); @@ -58,14 +25,9 @@ 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 new file mode 100644 index 000000000..a3df76492 --- /dev/null +++ b/packages/app/src/git/diff-flat-items.test.ts @@ -0,0 +1,134 @@ +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 new file mode 100644 index 000000000..cd60dd97c --- /dev/null +++ b/packages/app/src/git/diff-flat-items.ts @@ -0,0 +1,121 @@ +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 deleted file mode 100644 index c16be414b..000000000 --- a/packages/app/src/git/diff-horizontal-surface.tsx +++ /dev/null @@ -1,80 +0,0 @@ -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 a594faa9c..f0756b34d 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -1,5 +1,4 @@ import { - Fragment, useState, useCallback, useEffect, @@ -43,6 +42,7 @@ 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,10 +59,13 @@ 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, @@ -104,7 +107,6 @@ import { formatDiffContentText, formatDiffGutterText, hasVisibleDiffTokens, - compactHighlightTokens, } from "@/utils/diff-rendering"; import { isWeb, isNative } from "@/constants/platform"; import { useWorkspaceFileDragSource } from "@/attachments/use-workspace-file-drag-source"; @@ -120,15 +122,6 @@ 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"; @@ -183,6 +176,10 @@ function useDiffRowMetricsStyle(textMetricsStyle: TextStyle): StyleProp{token.text}; +} + function HighlightedText({ tokens, textMetricsStyle, @@ -199,21 +196,15 @@ function HighlightedText({ [textMetricsStyle, wrapLines], ); - 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]); + const keyedTokens = useMemo( + () => tokens.map((token, index) => ({ key: `${index}-${token.text}`, token })), + [tokens], + ); return ( {keyedTokens.map(({ key, token }) => ( - - {token.text} - + ))} ); @@ -308,7 +299,6 @@ function LongPressableLine({ hoverTargetKey, onHoverTargetChange, style, - testID, children, }: { reviewTarget: ReviewableDiffTarget | null | undefined; @@ -317,7 +307,6 @@ function LongPressableLine({ hoverTargetKey?: string | null; onHoverTargetChange?: (key: string | null) => void; style: StyleProp; - testID?: string; children: ReactNode; }) { const onStartComment = reviewActions?.onStartComment; @@ -347,26 +336,17 @@ function LongPressableLine({ if (isWeb && (onHoverChange || onHoverTargetChange)) { return ( - + {children} ); } if (!isNative || !reviewTarget || !onStartComment) { - return ( - - {children} - - ); + return {children}; } return ( - + {children} ); @@ -391,7 +371,6 @@ function DiffGutterCell({ style, textTestID, actionTestID, - rowTestID, }: { lineNumber: number | null; type: DiffLine["type"] | undefined | null; @@ -403,7 +382,6 @@ function DiffGutterCell({ style?: StyleProp; textTestID?: string; actionTestID?: string; - rowTestID?: string; }) { const lineHeight = getNumericLineHeight(textMetricsStyle); const rowMetricsStyle = useDiffRowMetricsStyle(textMetricsStyle); @@ -447,7 +425,6 @@ function DiffGutterCell({ onStartComment={onStartComment} style={containerStyle} actionTestID={actionTestID} - testID={rowTestID} > {formatDiffGutterText(lineNumber)} @@ -466,7 +443,6 @@ function DiffTextLine({ hoverTargetKey, onHoverTargetChange, textTestID, - rowTestID, }: { line: DiffLine; wrapLines: boolean; @@ -477,7 +453,6 @@ 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); @@ -508,7 +483,6 @@ function DiffTextLine({ hoverTargetKey={hoverTargetKey} onHoverTargetChange={onHoverTargetChange} style={containerStyle} - testID={rowTestID} > {line.type !== "header" && visibleTokens ? ( (null); const wrapCellStyle = useMemo( @@ -868,19 +835,16 @@ function SplitDiffColumn({ const linesContainerRowStyle = useMemo( () => [ styles.linesContainer, - inlineUnistylesStyle({ minWidth: Math.max(textViewportWidth, minContentWidth) }), + scrollWidth > 0 && inlineUnistylesStyle({ minWidth: scrollWidth }), ], - [minContentWidth, textViewportWidth], + [scrollWidth], ); const headerLineTextStyle = useMemo( () => [styles.diffTextMetrics, textMetricsStyle, styles.diffLineText, styles.headerLineText], [textMetricsStyle], ); - const keyedRows = useMemo( - () => rows.map((row, index) => ({ key: `row-${startRowIndex + index}`, row })), - [rows, startRowIndex], - ); + const keyedRows = useMemo(() => rows.map((row, i) => ({ key: `row-${i}`, row })), [rows]); if (wrapLines) { return ( @@ -968,47 +932,50 @@ function SplitDiffColumn({ ); })} - - {keyedRows.map(({ key, row }) => { - if (row.kind === "header") { + + {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 ( - - {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 ( - - - - - ); - })} - + })} + + ); } @@ -1287,251 +1254,194 @@ const DiffFileHeader = memo(function DiffFileHeader({ ); }); -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; - onHeightChange?: (key: string, height: number) => void; -} - -function DiffRowGroupView({ - item, - viewportWidth, +export function DiffFileBody({ + file, + layout, wrapLines, codeFontSize, textMetricsStyle, reviewActions, - onHeightChange, -}: DiffRowGroupProps) { + onBodyHeightChange, + testID, +}: { + file: ParsedDiffFile; + layout: "unified" | "split"; + 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); const [hoveredReviewTargetKey, setHoveredReviewTargetKey] = useState(null); + const { t } = useTranslation(); const handleLayout = useCallback( (event: LayoutChangeEvent) => { - if (wrapLines) { - onHeightChange?.(item.key, event.nativeEvent.layout.height); - } + setBodyWidth(event.nativeEvent.layout.width); + onBodyHeightChange?.(file, event.nativeEvent.layout.height); }, - [item.key, onHeightChange, wrapLines], + [file, onBodyHeightChange], ); - 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 availableWidth = bodyWidth > 0 ? bodyWidth : scrollViewWidth; const linesContainerRowStyle = useMemo( - () => [styles.linesContainer, inlineUnistylesStyle({ minWidth: minimumTextWidth })], - [minimumTextWidth], - ); - const groupStyle = useMemo( () => [ - styles.fileSectionBodyContainer, - item.isLast && styles.fileSectionBorder, - item.isFirst && styles.diffContent, - styles.diffGroupContent, + styles.linesContainer, + availableWidth > 0 && inlineUnistylesStyle({ minWidth: availableWidth }), ], - [item.isFirst, item.isLast], + [availableWidth], ); - if (item.type === "split_group") { - return ( - - - - - ); - } - - if (wrapLines) { - return ( - - - {item.lines.map(({ line, lineNumber, key, reviewTarget }, localIndex) => { - const index = item.startRowIndex + localIndex; - return ( - - - - - ); - })} - - - ); - } - return ( - - {item.lines.map(({ line, lineNumber, key, reviewTarget }, localIndex) => { - const index = item.startRowIndex + localIndex; - const hasReviewThread = - getInlineReviewThreadState({ reviewTarget, reviewActions }) !== null; + {(() => { + if (file.status === "too_large" || file.status === "binary") { 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} - + + ); - })} - - - {item.lines.map(({ line, key, reviewTarget }, localIndex) => { - const index = item.startRowIndex + localIndex; - const hasReviewThread = - getInlineReviewThreadState({ reviewTarget, reviewActions }) !== null; + } + + const computedLines = buildUnifiedDiffLines(file); + + if (wrapLines) { return ( - - - {hasReviewThread ? ( - - ) : null} - + + + {computedLines.map(({ line, lineNumber, key, reviewTarget }, index) => ( + + + + + ))} + + ); - })} - + } + + 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; @@ -1914,15 +1824,42 @@ export function DiffOptionsMenu({ const ThemedRotateCw = withUnistyles(RotateCw); -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; +type DiffFlatItemLayoutGetter = NonNullable["getItemLayout"]>; const EMPTY_PATH_LIST: string[] = []; -const EMPTY_NUMBER_LIST: number[] = []; + +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; +} function computeEmptyMessage( hideWhitespace: boolean, @@ -2061,9 +1998,6 @@ 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; @@ -2077,22 +2011,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ...(trimmedMonoFontFamily ? { fontFamily: trimmedMonoFontFamily } : null), }; }, [codeFontSize, diffBodyLineHeight, monoFontFamily]); - 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 viewMode = mode.kind === "working_tree" ? mode.viewMode : "flat"; const expandedPathsArray = useMemo(() => { if (mode.kind === "working_tree") { return mode.expandedPaths; @@ -2103,12 +2022,26 @@ 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); @@ -2129,24 +2062,19 @@ 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 measuredGroupHeightsRef = useRef(new Map()); - const rowCacheRef = useRef(createDiffRowCache()); + const bodyHeightByKeyRef = useRef>({}); 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; @@ -2156,106 +2084,82 @@ 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 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({ + const { flatItems, stickyHeaderIndices } = useMemo(() => { + const { items, stickyHeaderIndices: stickyIndices } = buildDiffFlatItems({ 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, }); - }, [ - 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], + 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], ); - const maximumGutterWidth = useMemo( - () => - files.reduce( - (maximum, file) => - Math.max(maximum, lineNumberGutterWidth(maxLineNumber(file), codeFontSize)), - 0, - ), - [codeFontSize, files], + + 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 horizontalCodeWidth = Math.ceil( - horizontalContentColumns * codeFontSize * 0.75 + SPACING[4], + + 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 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) => { @@ -2291,48 +2195,28 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi [scheduleHeightVersionUpdate], ); - const handleGroupHeightChange = useCallback( - (key: string, height: number) => { + const handleBodyHeightChange = useCallback( + (file: ParsedDiffFile, height: number) => { if (!Number.isFinite(height) || height < 0) { return; } - const previousHeight = measuredGroupHeightsRef.current.get(key); + const heightKey = getBodyHeightKey(file); + const previousHeight = bodyHeightByKeyRef.current[heightKey]; if ( previousHeight !== undefined && Math.abs(previousHeight - height) <= DIFF_HEIGHT_CHANGE_EPSILON ) { return; } - 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(); + bodyHeightByKeyRef.current[heightKey] = height; + scheduleHeightVersionUpdate(); }, - [scheduleGeometryVersionUpdate], + [getBodyHeightKey, scheduleHeightVersionUpdate], ); - 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) => { - const offset = event.nativeEvent.contentOffset.y; - diffListScrollOffsetRef.current = offset; + diffListScrollOffsetRef.current = event.nativeEvent.contentOffset.y; updateScrollbarOffset(event); }, [updateScrollbarOffset], @@ -2344,7 +2228,6 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi if (!Number.isFinite(height) || height <= 0) { return; } - setDiffListWidth(event.nativeEvent.layout.width); diffListViewportHeightRef.current = height; updateScrollbarLayout(event); }, @@ -2352,19 +2235,20 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ); const computeItemOffset = useCallback( - (predicate: (item: DiffListItem) => boolean): number | null => { + (predicate: (item: DiffFlatItem) => boolean): number | null => { const index = flatItems.findIndex(predicate); if (index < 0) { return null; } - return rowIndex.offsetAt(index); + return sumHeightsBefore(flatItems, index, getFlatItemHeight); }, - [flatItems, rowIndex], + [flatItems, getFlatItemHeight], ); const computeHeaderOffset = useCallback( - (path: string): number => rowIndex.offsetOfFile(path), - [rowIndex], + (path: string): number => + computeItemOffset((item) => item.type === "header" && item.file.path === path) ?? 0, + [computeItemOffset], ); useEffect(() => { @@ -2504,7 +2388,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ); const renderFlatItem = useCallback( - ({ item }: { item: DiffListItem }) => { + ({ item }: { item: DiffFlatItem }) => { if (item.type === "folder") { return ( ); } - if (item.type === "status") { - return ; - } return ( - ); }, [ codeFontSize, - diffListWidth, - handleGroupHeightChange, + handleBodyHeightChange, handleFolderRowHeightChange, handleHeaderHeightChange, handleCollapseFolder, handleSelectPath, handleToggleExpanded, handleToggleFolder, + layout, reviewActions, workspaceFileDragScope, textMetricsStyle, @@ -2600,26 +2482,20 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ], ); - 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 flatKeyExtractor = useCallback( + (item: DiffFlatItem) => + item.type === "folder" ? `folder-${item.dirPath}` : `${item.type}-${item.file.path}`, + [], + ); - const getFlatItemLayout = useCallback( + const getFlatItemLayout = useCallback( (_data, index) => { - void geometryVersion; - return { - length: rowIndex.heightAt(index), - offset: rowIndex.offsetAt(index), - index, - }; + const offset = sumHeightsBefore(flatItems, index, getFlatItemHeight); + const item = flatItems[index]; + const length = item ? getFlatItemHeight(item) : 0; + return { length, offset, index }; }, - [geometryVersion, rowIndex], + [flatItems, getFlatItemHeight], ); const flatExtraData = useMemo( @@ -2629,7 +2505,6 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi layout, typographyKey, heightVersion, - geometryVersion, viewMode, wrapLines, reviewActions, @@ -2639,7 +2514,6 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi expandedPathsArray, collapsedFoldersArray, heightVersion, - geometryVersion, layout, reviewActions, typographyKey, @@ -2649,45 +2523,29 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi ], ); - const diffList = ( - - ); - return ( - {wrapLines ? ( - diffList - ) : ( - - {diffList} - - )} + {scrollbar.overlay} ); @@ -3644,9 +3502,6 @@ const styles = StyleSheet.create((theme) => ({ borderTopColor: theme.colors.border, backgroundColor: theme.colors.surface1, }, - diffGroupContent: { - backgroundColor: theme.colors.surface1, - }, diffContentRow: { flexDirection: "row", alignItems: "stretch", @@ -3693,7 +3548,6 @@ 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 deleted file mode 100644 index bd74ffe24..000000000 --- a/packages/app/src/git/diff-rows/model.test.ts +++ /dev/null @@ -1,286 +0,0 @@ -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 deleted file mode 100644 index 0206c8aa6..000000000 --- a/packages/app/src/git/diff-rows/model.ts +++ /dev/null @@ -1,422 +0,0 @@ -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 deleted file mode 100644 index 6392b4912..000000000 --- a/packages/app/src/git/diff-rows/virtualized-list-range.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -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 7e80fd86c..fd79290b5 100644 --- a/packages/app/src/review/surface.tsx +++ b/packages/app/src/review/surface.tsx @@ -260,7 +260,6 @@ export function InlineReviewGutterCell({ onStartComment, style, actionTestID, - testID, }: { children: ReactNode; reviewTarget: ReviewableDiffTarget | null | undefined; @@ -271,7 +270,6 @@ export function InlineReviewGutterCell({ onStartComment: (target: ReviewableDiffTarget) => void; style?: StyleProp; actionTestID?: string; - testID?: string; }) { const { t } = useTranslation(); const canComment = Boolean(reviewTarget); @@ -311,6 +309,7 @@ export function InlineReviewGutterCell({ } }, [isInteractionActive]); + const pressableStyle = useCallback((): StyleProp => style, [style]); const lineHeightStyle = useMemo>( () => lineHeight !== undefined @@ -319,15 +318,13 @@ export function InlineReviewGutterCell({ [lineHeight], ); - const pressableStyle = useCallback( - (): StyleProp => [ - style, - styles.gutterInner, - styles.gutterLabel, - lineHeightStyle, - hasComments && styles.gutterLabelActive, - ], - [hasComments, lineHeightStyle, style], + const labelStyle = useMemo>( + () => [styles.gutterLabel, lineHeightStyle, hasComments && styles.gutterLabelActive], + [hasComments, lineHeightStyle], + ); + const innerStyle = useMemo>( + () => [styles.gutterInner, lineHeightStyle], + [lineHeightStyle], ); const actionIconStyle = useMemo>( () => [ @@ -349,14 +346,17 @@ export function InlineReviewGutterCell({ onPressIn={handlePressIn} onPressOut={handlePressOut} style={pressableStyle} - testID={testID} > - {children} - {showAction ? ( - - + + + {children} + {showAction ? ( + + + + ) : null} - ) : null} + ); } diff --git a/packages/app/src/types/virtualized-list-internals.d.ts b/packages/app/src/types/virtualized-list-internals.d.ts deleted file mode 100644 index ae357034c..000000000 --- a/packages/app/src/types/virtualized-list-internals.d.ts +++ /dev/null @@ -1,8 +0,0 @@ -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 384bd2291..4d6e35e89 100644 --- a/packages/app/src/utils/diff-rendering.test.ts +++ b/packages/app/src/utils/diff-rendering.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from "vitest"; import { - compactHighlightTokens, formatDiffContentText, formatDiffGutterText, hasVisibleDiffTokens, @@ -29,23 +28,4 @@ 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 c3cd9f8bb..a189c56b6 100644 --- a/packages/app/src/utils/diff-rendering.ts +++ b/packages/app/src/utils/diff-rendering.ts @@ -2,28 +2,6 @@ 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 deleted file mode 100644 index 0ab5e8abc..000000000 --- a/patches/@react-native+virtualized-lists+0.81.5.patch +++ /dev/null @@ -1,105 +0,0 @@ -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 465a78fc4..781294b6d 100644 --- a/scripts/postinstall-patches.mjs +++ b/scripts/postinstall-patches.mjs @@ -8,10 +8,6 @@ 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+",