diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index 81aa9038d8..998d292d44 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -397,23 +397,24 @@ test("every interactive file header has the same hover feedback", async ({ page const first = page.getByTestId("diff-file-0-toggle"); const second = page.getByTestId("diff-file-1-toggle"); - const canvas = page.getByTestId("git-diff-header-canvas"); - const normalBackground = await headerCanvasPixel(canvas, first, 10); + const firstCanvas = page.getByTestId("git-diff-sticky-header-0"); + const secondCanvas = page.getByTestId("git-diff-sticky-header-1"); + const normalBackground = await headerCanvasPixel(firstCanvas, first, 10); await first.hover(); - await expect.poll(() => headerCanvasPixel(canvas, first, 10)).not.toBe(normalBackground); - const hoverBackground = await headerCanvasPixel(canvas, first, 10); + await expect.poll(() => headerCanvasPixel(firstCanvas, first, 10)).not.toBe(normalBackground); + const hoverBackground = await headerCanvasPixel(firstCanvas, first, 10); await first.click(); await page.mouse.move(0, 0); await expect(first).toHaveAttribute("aria-expanded", "false"); - await expect.poll(() => headerCanvasPixel(canvas, first, 10)).toBe(normalBackground); + await expect.poll(() => headerCanvasPixel(firstCanvas, first, 10)).toBe(normalBackground); await first.hover(); - await expect.poll(() => headerCanvasPixel(canvas, first, 10)).toBe(hoverBackground); + await expect.poll(() => headerCanvasPixel(firstCanvas, first, 10)).toBe(hoverBackground); await second.hover(); - await expect.poll(() => headerCanvasPixel(canvas, first, 10)).toBe(normalBackground); - await expect.poll(() => headerCanvasPixel(canvas, second, 10)).toBe(hoverBackground); + await expect.poll(() => headerCanvasPixel(firstCanvas, first, 10)).toBe(normalBackground); + await expect.poll(() => headerCanvasPixel(secondCanvas, second, 10)).toBe(hoverBackground); }); test("horizontal body scrolling never moves or repaints the canvas header", async ({ page }) => { @@ -422,22 +423,28 @@ test("horizontal body scrolling never moves or repaints the canvas header", asyn await useUnwrappedDiffLines(page); await openSelectionWorkspaceChanges(page, workspace); - const headerCanvas = page.getByTestId("git-diff-header-canvas"); + const headerCanvas = page.getByTestId("git-diff-canvas"); const header = page.getByTestId("diff-file-0-toggle"); - const beforeImage = await headerCanvas.evaluate((element) => - (element as HTMLCanvasElement).toDataURL(), - ); + const beforePixel = await headerCanvasPixel(headerCanvas, header, 10); const beforeBounds = await header.boundingBox(); await horizontallyScrollFirstFile(page, 320); await page.waitForTimeout(50); - expect(await headerCanvas.evaluate((element) => (element as HTMLCanvasElement).toDataURL())).toBe( - beforeImage, - ); + expect(await headerCanvasPixel(headerCanvas, header, 10)).toBe(beforePixel); expect(await header.boundingBox()).toEqual(beforeBounds); }); +test("in-flow file headers move with the diff document", async ({ page }) => { + const workspace = await createWorkspaceWithStickyTransitionDiff(); + await useUnwrappedDiffLines(page); + await openWorkspaceChangesSurface(page, workspace); + + const motion = await measureInFlowHeaderMotion(page); + + expect(motion).toEqual({ headerSurface: -120, shell: -120 }); +}); + test("the outgoing sticky header hands off without a gap or overlap", async ({ page, }, testInfo) => { @@ -1096,7 +1103,7 @@ test("canvas headers keep a many-file diff bounded while scrolling end to end", const root = page.getByTestId("git-diff-canvas-root"); const scroller = page.getByTestId("git-diff-scroll"); - await expect(page.getByTestId("git-diff-header-canvas")).toBeVisible({ timeout: 30_000 }); + await expect(root.locator('[data-testid^="git-diff-sticky-header-"]')).toHaveCount(2); await expect.poll(() => root.locator('[data-diff-header="true"]').count()).toBeLessThan(120); await scroller.evaluate((element) => { @@ -1106,7 +1113,7 @@ test("canvas headers keep a many-file diff bounded while scrolling end to end", await expect(page.getByTestId("diff-file-1999")).toBeVisible(); await expect.poll(() => root.locator('[data-diff-header="true"]').count()).toBeLessThan(120); - await expect(page.getByTestId("git-diff-header-canvas")).toBeVisible(); + await expect(root.locator('[data-testid^="git-diff-sticky-header-"]')).toHaveCount(2); }); test("the whole reviewable row reveals the gutter affordance and uses a text cursor", async ({ @@ -2030,6 +2037,39 @@ async function horizontallyScrollFirstFile(page: Page, requestedOffset: number): return retainedOffset; } +async function measureInFlowHeaderMotion( + page: Page, +): Promise<{ headerSurface: number; shell: number }> { + return page.getByTestId("git-diff-scroll").evaluate(async (element) => { + const scroll = element as HTMLElement; + const headerSurface = document.querySelector('[data-testid="git-diff-canvas"]'); + const shell = document.querySelector('[data-diff-header-path="src/second.ts"]'); + if (!headerSurface || !shell) throw new Error("Diff header motion surfaces are unavailable"); + + const shellDocumentTop = + scroll.scrollTop + shell.getBoundingClientRect().top - scroll.getBoundingClientRect().top; + scroll.scrollTop = Math.max(0, shellDocumentTop - scroll.clientHeight + 80); + scroll.dispatchEvent(new Event("scroll", { bubbles: false })); + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())), + ); + + const before = { + headerSurface: headerSurface.getBoundingClientRect().top, + shell: shell.getBoundingClientRect().top, + }; + scroll.scrollTop += 120; + const after = { + headerSurface: headerSurface.getBoundingClientRect().top, + shell: shell.getBoundingClientRect().top, + }; + return { + headerSurface: after.headerSurface - before.headerSurface, + shell: after.shell - before.shell, + }; + }); +} + async function longPressFileHeader(page: Page, header: Locator): Promise { const bounds = await header.boundingBox(); if (!bounds) throw new Error("File header has no bounds"); diff --git a/packages/app/e2e/browser/commit-diff-panel.spec.ts b/packages/app/e2e/browser/commit-diff-panel.spec.ts index 400ba59754..d29e566f80 100644 --- a/packages/app/e2e/browser/commit-diff-panel.spec.ts +++ b/packages/app/e2e/browser/commit-diff-panel.spec.ts @@ -105,7 +105,7 @@ async function createFeatureCommit(repoPath: string): Promise { async function expectCommitDiffHeaderGeometry(panel: Locator): Promise { const [header, canvas] = await Promise.all([ panel.getByTestId("diff-file-0").boundingBox(), - panel.getByTestId("git-diff-header-canvas").boundingBox(), + panel.getByTestId("git-diff-sticky-header-0").boundingBox(), ]); expect(header).not.toBeNull(); expect(canvas).not.toBeNull(); diff --git a/packages/app/e2e/browser/diff-performance.spec.ts b/packages/app/e2e/browser/diff-performance.spec.ts index b0c5a3bdfc..c11c82f279 100644 --- a/packages/app/e2e/browser/diff-performance.spec.ts +++ b/packages/app/e2e/browser/diff-performance.spec.ts @@ -93,7 +93,10 @@ diffPerfDescribe("Diff canvas performance", () => { [ root.querySelectorAll('[data-diff-header="true"]').length, root.querySelectorAll("*").length, - root.querySelector('[data-testid="git-diff-header-canvas"]') ? "canvas" : "dom", + root.querySelector('[data-testid="git-diff-canvas"]') && + root.querySelectorAll('[data-testid^="git-diff-sticky-header-"]').length === 2 + ? "canvas-pool" + : "dom", ] as const, ); const headerDomMetrics = await page.getByTestId("diff-file-0-toggle").evaluate((header) => { @@ -125,7 +128,7 @@ diffPerfDescribe("Diff canvas performance", () => { }; }); const headerCanvasMetrics = await page - .getByTestId("git-diff-header-canvas") + .getByTestId("git-diff-sticky-header-0") .evaluate((canvas: HTMLCanvasElement) => { const context = canvas.getContext("2d"); if (!context) throw new Error("Header canvas has no 2D context"); @@ -168,6 +171,7 @@ diffPerfDescribe("Diff canvas performance", () => { ); if (!RUN_HEADER_COMPARISON) { expect(benchmark.maximumShells).toBeLessThan(120); + expect(benchmark.maximumStickyCanvases).toBe(2); expect(benchmark.blankHeaderFrames).toBe(0); expect(benchmark.medianFrameMs).toBeLessThanOrEqual(MANY_FILE_MAX_MEDIAN_FRAME_MS); expect(benchmark.p95FrameMs).toBeLessThanOrEqual(MANY_FILE_MAX_P95_FRAME_MS); @@ -435,7 +439,8 @@ async function openFirstReviewEditor(page: Page): Promise { } async function expectBoundedCanvasSurfaces(page: Page): Promise { - await expect(page.locator("canvas")).toHaveCount(2); + await expect(page.locator("canvas")).toHaveCount(3); + await expect(page.locator('[data-testid^="git-diff-sticky-header-"]')).toHaveCount(2); await expect(page.locator('[data-testid^="diff-code-row-"]')).toHaveCount(0); const elementCount = await page .getByTestId("git-diff-canvas-root") @@ -527,40 +532,57 @@ async function benchmarkManyFileHeaderScrolling(page: Page): Promise<{ p95FrameMs: number; maximumFrameMs: number; maximumShells: number; + maximumStickyCanvases: number; blankHeaderFrames: number; endScrollTop: number; maximumScrollTop: number; }> { return page.getByTestId("git-diff-scroll").evaluate(async (element) => { const scroll = element as HTMLElement; - const canvas = document.querySelector( - '[data-testid="git-diff-header-canvas"]', - ); const maximumScrollTop = scroll.scrollHeight - scroll.clientHeight; const frameDurations: number[] = []; let maximumShells = 0; + let maximumStickyCanvases = 0; let blankHeaderFrames = 0; const sampleHeader = () => { maximumShells = Math.max( maximumShells, document.querySelectorAll('[data-diff-header="true"]').length, ); + const canvases = Array.from( + document.querySelectorAll('[data-testid^="git-diff-sticky-header-"]'), + ); + maximumStickyCanvases = Math.max(maximumStickyCanvases, canvases.length); + const scrollBounds = scroll.getBoundingClientRect(); + const sampleX = scrollBounds.left + 4; + const sampleY = scrollBounds.top + 10; + const canvas = canvases.find((candidate) => { + const bounds = candidate.getBoundingClientRect(); + return ( + bounds.width > 0 && + sampleX >= bounds.left && + sampleX < bounds.right && + sampleY >= bounds.top && + sampleY < bounds.bottom + ); + }); if (canvas) { const context = canvas.getContext("2d")!; - const ratio = canvas.width / canvas.getBoundingClientRect().width; + const bounds = canvas.getBoundingClientRect(); + const ratio = canvas.width / bounds.width; if ( - context.getImageData(Math.round(4 * ratio), Math.round(10 * ratio), 1, 1).data[3] === 0 + context.getImageData( + Math.round((sampleX - bounds.left) * ratio), + Math.round((sampleY - bounds.top) * ratio), + 1, + 1, + ).data[3] === 0 ) { blankHeaderFrames += 1; } return; } - const bounds = scroll.getBoundingClientRect(); - if ( - !document.elementFromPoint(bounds.left + 4, bounds.top + 10)?.closest("[data-diff-header]") - ) { - blankHeaderFrames += 1; - } + blankHeaderFrames += 1; }; for (const progress of [0, 0.25, 0.5, 0.75, 1]) { @@ -601,6 +623,7 @@ async function benchmarkManyFileHeaderScrolling(page: Page): Promise<{ p95FrameMs: percentile(0.95), maximumFrameMs: Math.max(...frameDurations), maximumShells, + maximumStickyCanvases, blankHeaderFrames, endScrollTop: scroll.scrollTop, maximumScrollTop, diff --git a/packages/app/src/git/diff-document/header-layout.test.ts b/packages/app/src/git/diff-document/header-layout.test.ts index 349ca9c7d3..bdfdf88fff 100644 --- a/packages/app/src/git/diff-document/header-layout.test.ts +++ b/packages/app/src/git/diff-document/header-layout.test.ts @@ -1,9 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - diffHeaderViewportKey, - diffInteractionWindowTop, - resolveVisibleFileSections, -} from "./header-layout"; +import { diffInteractionWindowTop, resolveVisibleFileSections } from "./header-layout"; import type { DiffFileSection } from "./types"; describe("diff header viewport", () => { @@ -48,15 +44,6 @@ describe("diff header viewport", () => { ?.y, ).toBe(0); }); - - it("reuses a fixed sticky header until its visible layout changes", () => { - const files = [section(0, 0, 4_000, false), section(1, 4_000, 4_030, true)]; - const keyAt = (scrollTop: number) => - diffHeaderViewportKey({ files, scrollTop, viewportHeight: 600, activePath: null }); - - expect(keyAt(100)).toBe(keyAt(2_000)); - expect(keyAt(3_985)).not.toBe(keyAt(2_000)); - }); }); describe("diff interaction window", () => { diff --git a/packages/app/src/git/diff-document/header-layout.ts b/packages/app/src/git/diff-document/header-layout.ts index c53ba1867d..ffed4ec4b3 100644 --- a/packages/app/src/git/diff-document/header-layout.ts +++ b/packages/app/src/git/diff-document/header-layout.ts @@ -21,20 +21,6 @@ export function diffMaterializationWindow( }; } -export function diffHeaderViewportKey(input: { - files: readonly DiffFileSection[]; - scrollTop: number; - viewportHeight: number; - activePath: string | null; -}): string { - const window = resolveVisibleFileSections({ ...input, overscan: 0 }); - const headers = window.files - .filter((file) => file !== window.sticky?.file) - .map((file) => `${file.fileIndex}:${file.top - input.scrollTop}`); - if (window.sticky) headers.push(`${window.sticky.file.fileIndex}:${window.sticky.y}`); - return `${input.activePath ?? ""}|${headers.join(",")}`; -} - export function resolveVisibleFileSections(input: { files: readonly DiffFileSection[]; scrollTop: number; diff --git a/packages/app/src/git/diff-document/paint.web.test.ts b/packages/app/src/git/diff-document/paint.web.test.ts index aa9640a5d7..5d333f01cb 100644 --- a/packages/app/src/git/diff-document/paint.web.test.ts +++ b/packages/app/src/git/diff-document/paint.web.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { paintWebHeaders, paintWebViewport } from "./paint.web"; +import { paintWebViewport } from "./paint.web"; import type { DiffCell, DiffDocumentModel, DiffLineRow, DiffPalette, DiffSelection } from "./types"; describe("web diff text shaping", () => { @@ -53,7 +53,7 @@ describe("web diff text shaping", () => { lineJoin: "miter", } as unknown as CanvasRenderingContext2D; - paintWebHeaders({ + paintWebViewport({ context, model: { ...model, @@ -69,19 +69,25 @@ describe("web diff text shaping", () => { height: 48, }, palette, - typography: { family: "system-ui", size: 14, statSize: 12 }, + typography: { family: "monospace", size: 12, lineHeight: 18 }, + headerTypography: { family: "system-ui", size: 14, statSize: 12 }, + measureText: { measure: () => 0 }, scrollTop: 0, viewportWidth: 200, viewportHeight: 100, + horizontalOffsets: new Map(), + selection: null, devicePixelRatio: 1, - activePath: null, + activeHeaderPath: null, }); - expect(fills.slice(0, 2)).toEqual([ + expect( + fills.filter((fill) => fill.color === "header" || fill.color === "header-border"), + ).toEqual([ { color: "header", x: 0, y: 0, width: 200, height: 30 }, { color: "header-border", x: 0, y: 29, width: 200, height: 1 }, ]); - expect(labels).toEqual([ + expect(labels.filter((label) => label.text !== "fi")).toEqual([ { text: "+1", x: 142, y: 18, color: "success", font: "12px system-ui" }, { text: "-0", x: 158, y: 18, color: "danger", font: "12px system-ui" }, { text: "a.ts", x: 12, y: 18, color: "foreground", font: "14px system-ui" }, @@ -114,12 +120,14 @@ describe("web diff text shaping", () => { model, palette, typography: { family: "ligature-font", size: 12, lineHeight: 18 }, + headerTypography, measureText: { measure: () => 0 }, scrollTop: 0, viewportWidth: 200, viewportHeight: 100, horizontalOffsets: new Map(), selection: null, + activeHeaderPath: null, devicePixelRatio: 1, }); @@ -157,12 +165,14 @@ describe("web diff text shaping", () => { model: modelWithReview, palette, typography: { family: "monospace", size: 12, lineHeight: 18 }, + headerTypography, measureText: { measure: () => 0 }, scrollTop: 0, viewportWidth: 200, viewportHeight: 100, horizontalOffsets: new Map(), selection: null, + activeHeaderPath: null, devicePixelRatio: 1, }); @@ -220,12 +230,14 @@ describe("web diff text shaping", () => { model: borderedModel, palette, typography: { family: "monospace", size: 12, lineHeight: 18 }, + headerTypography, measureText: { measure: () => 0 }, scrollTop: 0, viewportWidth: 200, viewportHeight: 100, horizontalOffsets: new Map(), selection: null, + activeHeaderPath: null, devicePixelRatio: 1, }); @@ -331,12 +343,14 @@ function paintSelection(model: DiffDocumentModel, selection: DiffSelection): Sel model, palette, typography: { family: "monospace", size: 12, lineHeight: 18 }, + headerTypography, measureText: { measure: () => 0 }, scrollTop: 0, viewportWidth: 200, viewportHeight: 100, horizontalOffsets: new Map([["src/selection.ts", 50]]), selection, + activeHeaderPath: null, devicePixelRatio: 1, }); return paints; @@ -451,6 +465,8 @@ const palette: DiffPalette = { syntax: { first: "red", second: "blue" }, }; +const headerTypography = { family: "system-ui", size: 14, statSize: 12 }; + const model: DiffDocumentModel = { files: [ { diff --git a/packages/app/src/git/diff-document/paint.web.ts b/packages/app/src/git/diff-document/paint.web.ts index 2ceb3de4f3..f94e30d67e 100644 --- a/packages/app/src/git/diff-document/paint.web.ts +++ b/packages/app/src/git/diff-document/paint.web.ts @@ -19,7 +19,6 @@ import { fileNameForPath, formatDiffCount, } from "@/git/file-header-presentation"; -import { resolveVisibleFileSections } from "./header-layout"; import { reviewBackgroundPaint, reviewDividerHeight, reviewGapTop } from "./review-paint"; import type { DiffCell, @@ -40,12 +39,14 @@ export interface PaintWebViewportInput { model: DiffDocumentModel; palette: DiffPalette; typography: DiffTypography; + headerTypography: DiffHeaderTypography; measureText: TextMeasurer; scrollTop: number; viewportWidth: number; viewportHeight: number; horizontalOffsets: ReadonlyMap; selection: DiffSelection | null; + activeHeaderPath: string | null; devicePixelRatio: number; paintTop?: number; paintHeight?: number; @@ -98,42 +99,38 @@ export function paintWebViewport(input: PaintWebViewportInput): void { context.fillRect(0, borderTop - input.scrollTop, input.viewportWidth, DIFF_BODY_BORDER_HEIGHT); } + for (const file of input.model.files) { + if ( + file.headerHeight <= 0 || + file.top + file.headerHeight <= paintDocumentTop || + file.top >= paintDocumentBottom + ) { + continue; + } + paintWebFileHeader({ + context, + file, + palette: input.palette, + typography: input.headerTypography, + viewportWidth: input.viewportWidth, + y: file.top - input.scrollTop, + activePath: input.activeHeaderPath, + }); + } + if (input.selection) paintSelection(input, input.selection); context.restore(); } -export function paintWebHeaders(input: { +export function paintWebFileHeader(input: { context: CanvasRenderingContext2D; - model: DiffDocumentModel; + file: DiffFileSection; palette: DiffPalette; typography: DiffHeaderTypography; - scrollTop: number; viewportWidth: number; - viewportHeight: number; - devicePixelRatio: number; + y: number; activePath: string | null; }): void { - const scale = input.devicePixelRatio; - input.context.setTransform(scale, 0, 0, scale, 0, 0); - input.context.clearRect(0, 0, input.viewportWidth, input.viewportHeight); - const window = resolveVisibleFileSections({ - files: input.model.files, - scrollTop: input.scrollTop, - viewportHeight: input.viewportHeight, - overscan: 0, - }); - for (const file of window.files) { - if (file === window.sticky?.file) continue; - const y = file.top - input.scrollTop; - if (y + file.headerHeight <= 0 || y >= input.viewportHeight) continue; - paintWebHeader({ ...input, file, y }); - } - if (window.sticky) paintWebHeader({ ...input, file: window.sticky.file, y: window.sticky.y }); -} - -function paintWebHeader( - input: Parameters[0] & { file: DiffFileSection; y: number }, -): void { const { context, file, palette, typography, viewportWidth, y } = input; context.fillStyle = input.activePath === file.path ? palette.headerActiveSurface : palette.headerSurface; diff --git a/packages/app/src/git/diff-document/surface.web.tsx b/packages/app/src/git/diff-document/surface.web.tsx index 0b24be0610..bcabe0ee4f 100644 --- a/packages/app/src/git/diff-document/surface.web.tsx +++ b/packages/app/src/git/diff-document/surface.web.tsx @@ -15,7 +15,6 @@ import { copyToClipboard } from "@/utils/copy-to-clipboard"; import type { ReviewableDiffTarget } from "@/utils/diff-layout"; import { DocumentFileHeader } from "./document-file-header"; import { - diffHeaderViewportKey, diffInteractionWindowTop, diffMaterializationWindow, resolveVisibleFileSections, @@ -29,12 +28,13 @@ import { import { retainHorizontalOffsetMapForPaths } from "./horizontal-offsets"; import { HorizontalScroll } from "./horizontal-scroll.web"; import { buildDiffDocumentModel, FILE_HEADER_HEIGHT, resolveRelayoutScrollTop } from "./model"; -import { paintWebHeaders, paintWebViewport } from "./paint.web"; +import { paintWebFileHeader, paintWebViewport } from "./paint.web"; import { hasPointerDragStarted } from "./pointer-gesture"; import { createMeasuredAdvances } from "./text-measurement"; import { retainDiffViewport } from "./viewport"; import type { DiffHit, + DiffFileSection, DiffSelection, DiffSurfaceProps, DiffTypography, @@ -45,13 +45,40 @@ import { useDiffDocumentWorkspaceCache } from "./workspace-cache"; const DEFAULT_MONO_STACK = "ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace"; const RESIZE_SETTLE_DELAY_MS = 120; +interface StickyHeaderCanvasSlot { + section: HTMLDivElement | null; + canvas: HTMLCanvasElement | null; + file: DiffFileSection | null; + active: boolean; + width: number; + ratio: number; + palette: DiffSurfaceProps["palette"] | null; + typography: DiffSurfaceProps["headerTypography"] | null; +} + +function emptyStickyHeaderCanvasSlot(): StickyHeaderCanvasSlot { + return { + section: null, + canvas: null, + file: null, + active: false, + width: 0, + ratio: 0, + palette: null, + typography: null, + }; +} + export function DiffSurface(props: DiffSurfaceProps) { const { t } = useTranslation(); const toast = useToast(); const workspaceCache = useDiffDocumentWorkspaceCache(); const rootRef = useRef(null); const canvasRef = useRef(null); - const headerCanvasRef = useRef(null); + const stickyHeaderSlotsRef = useRef<[StickyHeaderCanvasSlot, StickyHeaderCanvasSlot]>([ + emptyStickyHeaderCanvasSlot(), + emptyStickyHeaderCanvasSlot(), + ]); const canvasScratchRef = useRef(null); const scrollRef = useRef(null); const modelRef = useRef | null>(null); @@ -68,16 +95,6 @@ export function DiffSurface(props: DiffSurfaceProps) { dismissSelectionOnClick: boolean; } | null>(null); const frameRef = useRef(null); - const headerFrameRef = useRef(null); - const headerPaintRef = useRef<{ - key: string; - model: ReturnType; - width: number; - height: number; - ratio: number; - palette: DiffSurfaceProps["palette"]; - typography: DiffSurfaceProps["headerTypography"]; - } | null>(null); const activeHeaderPathRef = useRef(null); const resizeSettleTimerRef = useRef | null>(null); const resizeReleaseFrameRef = useRef(null); @@ -185,6 +202,75 @@ export function DiffSurface(props: DiffSurfaceProps) { ]); modelRef.current = model; + const paintStickyHeaderPool = useCallback( + (currentModel: ReturnType, scrollTop: number) => { + const stickyFile = resolveVisibleFileSections({ + files: currentModel.files, + scrollTop, + viewportHeight: Math.max(1, viewport.height), + overscan: 0, + }).sticky?.file; + const pooledFiles = stickyFile + ? [stickyFile, currentModel.files[stickyFile.fileIndex + 1]].filter( + (file): file is DiffFileSection => file !== undefined, + ) + : []; + const ratio = window.devicePixelRatio || 1; + + for (const [slotIndex, slot] of stickyHeaderSlotsRef.current.entries()) { + const file = pooledFiles.find((candidate) => candidate.fileIndex % 2 === slotIndex) ?? null; + const { section, canvas } = slot; + if (!section || !canvas) continue; + if (!file) { + section.style.display = "none"; + slot.file = null; + continue; + } + + section.style.display = "block"; + section.style.top = `${file.top}px`; + section.style.height = `${file.bottom - file.top}px`; + section.dataset.diffStickyHeaderPath = file.path; + const active = activeHeaderPathRef.current === file.path; + const canReusePaint = + slot.file === file && + slot.active === active && + slot.width === viewport.width && + slot.ratio === ratio && + slot.palette === props.palette && + slot.typography === props.headerTypography; + if (canReusePaint) continue; + + const pixelWidth = Math.ceil(viewport.width * ratio); + const pixelHeight = Math.ceil(FILE_HEADER_HEIGHT * ratio); + if (canvas.width !== pixelWidth) canvas.width = pixelWidth; + if (canvas.height !== pixelHeight) canvas.height = pixelHeight; + canvas.style.width = `${viewport.width}px`; + canvas.style.height = `${FILE_HEADER_HEIGHT}px`; + const context = canvas.getContext("2d"); + if (!context) continue; + context.setTransform(ratio, 0, 0, ratio, 0, 0); + context.clearRect(0, 0, viewport.width, FILE_HEADER_HEIGHT); + paintWebFileHeader({ + context, + file, + palette: props.palette, + typography: props.headerTypography, + viewportWidth: viewport.width, + y: 0, + activePath: activeHeaderPathRef.current, + }); + slot.file = file; + slot.active = active; + slot.width = viewport.width; + slot.ratio = ratio; + slot.palette = props.palette; + slot.typography = props.headerTypography; + } + }, + [props.headerTypography, props.palette, viewport.height, viewport.width], + ); + const paint = useCallback(() => { frameRef.current = null; const canvas = canvasRef.current; @@ -244,17 +330,19 @@ export function DiffSurface(props: DiffSurfaceProps) { model: currentModel, palette: props.palette, typography: loadedTypography, + headerTypography: props.headerTypography, measureText: measurement, scrollTop: canvasTop, viewportWidth: currentModel.viewportWidth, viewportHeight: canvasHeight, horizontalOffsets: horizontalOffsetsRef.current, selection: selectionRef.current, + activeHeaderPath: activeHeaderPathRef.current, devicePixelRatio: ratio, paintTop, paintHeight, }); - }, [loadedTypography, measurement, props.palette, viewport.height]); + }, [loadedTypography, measurement, props.headerTypography, props.palette, viewport.height]); const schedulePaint = useCallback( (force = true) => { if (force) forcePaintRef.current = true; @@ -262,65 +350,6 @@ export function DiffSurface(props: DiffSurfaceProps) { }, [paint], ); - const paintHeaders = useCallback(() => { - headerFrameRef.current = null; - const canvas = headerCanvasRef.current; - const currentModel = modelRef.current; - if (!canvas || !currentModel || viewport.width <= 0 || viewport.height <= 0) return; - const ratio = window.devicePixelRatio || 1; - const key = diffHeaderViewportKey({ - files: currentModel.files, - scrollTop: scrollTopRef.current, - viewportHeight: viewport.height, - activePath: activeHeaderPathRef.current, - }); - const previous = headerPaintRef.current; - if ( - previous?.key === key && - previous.model === currentModel && - previous.width === viewport.width && - previous.height === viewport.height && - previous.ratio === ratio && - previous.palette === props.palette && - previous.typography === props.headerTypography - ) { - return; - } - headerPaintRef.current = { - key, - model: currentModel, - width: viewport.width, - height: viewport.height, - ratio, - palette: props.palette, - typography: props.headerTypography, - }; - const pixelWidth = Math.ceil(viewport.width * ratio); - const pixelHeight = Math.ceil(viewport.height * ratio); - if (canvas.width !== pixelWidth) canvas.width = pixelWidth; - if (canvas.height !== pixelHeight) canvas.height = pixelHeight; - canvas.style.width = `${viewport.width}px`; - canvas.style.height = `${viewport.height}px`; - const context = canvas.getContext("2d"); - if (!context) return; - paintWebHeaders({ - context, - model: currentModel, - palette: props.palette, - typography: props.headerTypography, - scrollTop: scrollTopRef.current, - viewportWidth: viewport.width, - viewportHeight: viewport.height, - devicePixelRatio: ratio, - activePath: activeHeaderPathRef.current, - }); - }, [props.headerTypography, props.palette, viewport.height, viewport.width]); - const scheduleHeaderPaint = useCallback(() => { - if (headerFrameRef.current === null) { - headerFrameRef.current = requestAnimationFrame(paintHeaders); - } - }, [paintHeaders]); - useLayoutEffect(() => { const root = rootRef.current; if (!root) return; @@ -427,17 +456,15 @@ export function DiffSurface(props: DiffSurfaceProps) { ); useLayoutEffect(() => { schedulePaint(); - scheduleHeaderPaint(); + paintStickyHeaderPool(model, scrollTopRef.current); updateInteractionFiles(scrollTopRef.current); - }, [model, scheduleHeaderPaint, schedulePaint, updateInteractionFiles]); + }, [model, paintStickyHeaderPool, schedulePaint, updateInteractionFiles]); useEffect( () => () => { if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); - if (headerFrameRef.current !== null) cancelAnimationFrame(headerFrameRef.current); // Fast Refresh preserves refs while rerunning effects. Release ownership of the canceled // request so the refreshed renderer can schedule its first paint. frameRef.current = null; - headerFrameRef.current = null; }, [], ); @@ -462,14 +489,18 @@ export function DiffSurface(props: DiffSurfaceProps) { (scrollElement: HTMLDivElement) => { const scrollTop = scrollElement.scrollTop; scrollTopRef.current = scrollTop; - if (activeHeaderPathRef.current !== null) activeHeaderPathRef.current = null; - scheduleHeaderPaint(); + const clearedActiveHeader = activeHeaderPathRef.current !== null; + if (clearedActiveHeader) { + activeHeaderPathRef.current = null; + schedulePaint(); + } updateInteractionFiles(scrollTop); if (hasHoveredAffordanceRef.current) { hasHoveredAffordanceRef.current = false; setHoveredAffordance(null); } const currentModel = modelRef.current; + if (currentModel) paintStickyHeaderPool(currentModel, scrollTop); const currentWindow = canvasWindowRef.current; if (!currentModel || currentWindow.height === 0) { schedulePaint(false); @@ -489,7 +520,7 @@ export function DiffSurface(props: DiffSurfaceProps) { const nextTop = Math.round(requestedTop * ratio) / ratio; if (nextTop !== currentWindow.top) schedulePaint(false); }, - [scheduleHeaderPaint, schedulePaint, updateInteractionFiles, viewport.height], + [paintStickyHeaderPool, schedulePaint, updateInteractionFiles, viewport.height], ); useEffect(() => { const scroll = scrollRef.current; @@ -580,9 +611,11 @@ export function DiffSurface(props: DiffSurfaceProps) { const activeHeaderPath = header?.dataset.diffHeaderPath ?? null; if (activeHeaderPathRef.current === activeHeaderPath) return; activeHeaderPathRef.current = activeHeaderPath; - scheduleHeaderPaint(); + const currentModel = modelRef.current; + if (currentModel) paintStickyHeaderPool(currentModel, scrollTopRef.current); + schedulePaint(); }, - [scheduleHeaderPaint], + [paintStickyHeaderPool, schedulePaint], ); const pointerMove = useCallback( (event: React.PointerEvent) => { @@ -780,6 +813,8 @@ export function DiffSurface(props: DiffSurfaceProps) { >
+ + {interactionFiles.map((file) => (
- {hoveredAffordance?.hit.target && reviewActions ? ( @@ -872,6 +902,36 @@ export function DiffSurface(props: DiffSurfaceProps) { ); } +function StickyHeaderCanvasSlotView({ + slotsRef, + slotIndex, +}: { + slotsRef: React.RefObject<[StickyHeaderCanvasSlot, StickyHeaderCanvasSlot]>; + slotIndex: 0 | 1; +}) { + const setSection = useCallback( + (section: HTMLDivElement | null) => { + slotsRef.current[slotIndex].section = section; + }, + [slotIndex, slotsRef], + ); + const setCanvas = useCallback( + (canvas: HTMLCanvasElement | null) => { + slotsRef.current[slotIndex].canvas = canvas; + }, + [slotIndex, slotsRef], + ); + return ( +
+ +
+ ); +} + function shiftCanvasPixels(input: { canvas: HTMLCanvasElement; scratch: HTMLCanvasElement; @@ -1029,6 +1089,20 @@ const FILE_SECTION_STYLE: React.CSSProperties = { pointerEvents: "none", userSelect: "none", }; +const STICKY_CANVAS_SECTION_STYLE: React.CSSProperties = { + position: "absolute", + display: "none", + left: 0, + right: 0, + zIndex: 2, + pointerEvents: "none", +}; +const STICKY_CANVAS_STYLE: React.CSSProperties = { + position: "sticky", + top: 0, + display: "block", + pointerEvents: "none", +}; const STICKY_HEADER_STYLE: React.CSSProperties = { position: "sticky", top: 0, @@ -1051,12 +1125,6 @@ const CANVAS_STYLE: React.CSSProperties = { zIndex: 1, pointerEvents: "none", }; -const HEADER_CANVAS_STYLE: React.CSSProperties = { - position: "absolute", - inset: 0, - zIndex: 3, - pointerEvents: "none", -}; const AFFORDANCE_STYLE: ViewStyle = { position: "absolute", zIndex: 5,