From b362af6e323a7482ba3fe8543c911082f1f31832 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 17 Aug 2026 14:34:19 +0200 Subject: [PATCH] fix(app): prevent diff canvas resize stretching Keep the last painted canvas width while a pane resize is active, then repaint during layout before the settled width reaches the screen. This avoids scaling the bitmap during the drag or on its final handoff frame. --- packages/app/e2e/browser/changes-pane.spec.ts | 76 +++++++++++++++++++ .../app/src/git/diff-document/surface.web.tsx | 68 ++++++++++++++--- packages/app/src/git/diff-pane.tsx | 16 +++- 3 files changed, 146 insertions(+), 14 deletions(-) diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index 8d8c9c2d5..ae29e73c3 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -725,6 +725,82 @@ test("desktop Changes toggles a navigation tree beside the expanded diff documen await expectFlatFileList(page); }); +test("canvas diff stays sharp while its workspace pane is resized", async ({ page }) => { + const workspace = await createWorkspaceWithMountedTabDiff(); + await useUnwrappedDiffLines(page); + await openWorkspaceChanges(page, workspace); + + const canvas = page.getByTestId("git-diff-canvas"); + const root = page.getByTestId("git-diff-canvas-root"); + const handle = page.getByTestId("workspace-split-resize-handle").getByRole("separator"); + await expect(handle).toBeVisible(); + await expect + .poll(async () => { + const [canvasWidth, rootWidth] = await Promise.all([ + canvas.evaluate((element) => (element as HTMLCanvasElement).getBoundingClientRect().width), + root.evaluate((element) => element.getBoundingClientRect().width), + ]); + return Math.abs(canvasWidth - rootWidth) < 1; + }) + .toBe(true); + const [handleBounds, before] = await Promise.all([ + handle.boundingBox(), + canvas.evaluate((element) => { + const canvasElement = element as HTMLCanvasElement; + return { + width: canvasElement.getBoundingClientRect().width, + ratio: window.devicePixelRatio || 1, + }; + }), + ]); + if (!handleBounds) throw new Error("Workspace split resize handle has no bounds"); + + await page.mouse.move(handleBounds.x + handleBounds.width / 2, handleBounds.y + 120); + await page.mouse.down(); + await page.mouse.move(handleBounds.x - 120, handleBounds.y + 120); + await page.evaluate(() => new Promise((resolve) => requestAnimationFrame(() => resolve()))); + + const duringDrag = await Promise.all([ + canvas.evaluate((element) => (element as HTMLCanvasElement).getBoundingClientRect().width), + root.evaluate((element) => element.getBoundingClientRect().width), + ]); + expect(duringDrag[0]).toBeCloseTo(before.width, 0); + expect(duringDrag[1]).toBeGreaterThan(before.width + 10); + + await page.mouse.up(); + const resizeFrames = await page.evaluate(async () => { + const canvasElement = document.querySelector( + '[data-testid="git-diff-canvas"]', + )!; + const ratio = window.devicePixelRatio || 1; + const frames: Array<{ cssWidth: number; bitmapWidth: number }> = []; + for (let index = 0; index < 10; index += 1) { + await new Promise((resolve) => requestAnimationFrame(() => resolve())); + frames.push({ + cssWidth: canvasElement.getBoundingClientRect().width, + bitmapWidth: canvasElement.width / ratio, + }); + } + return frames; + }); + for (const frame of resizeFrames) { + expect(frame.cssWidth).toBeCloseTo(frame.bitmapWidth, 0); + } + await expect + .poll(async () => { + const [canvasWidth, rootWidth, backingWidth] = await Promise.all([ + canvas.evaluate((element) => (element as HTMLCanvasElement).getBoundingClientRect().width), + root.evaluate((element) => element.getBoundingClientRect().width), + canvas.evaluate((element) => (element as HTMLCanvasElement).width), + ]); + return ( + Math.abs(canvasWidth - rootWidth) < 1 && + Math.abs(backingWidth / before.ratio - rootWidth) < 1 + ); + }) + .toBe(true); +}); + test("changes diff applies code size changes to gutter and code typography", async ({ page }) => { const workspace = await createWorkspaceWithMountedTabDiff(); await useCodeFont(page, 12); diff --git a/packages/app/src/git/diff-document/surface.web.tsx b/packages/app/src/git/diff-document/surface.web.tsx index ae1ad9e51..a0c448d3b 100644 --- a/packages/app/src/git/diff-document/surface.web.tsx +++ b/packages/app/src/git/diff-document/surface.web.tsx @@ -25,6 +25,7 @@ import type { } from "./types"; const DEFAULT_MONO_STACK = "ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace"; +const RESIZE_SETTLE_DELAY_MS = 120; export function DiffSurface(props: DiffSurfaceProps) { const { t } = useTranslation(); @@ -49,6 +50,10 @@ export function DiffSurface(props: DiffSurfaceProps) { moved: boolean; } | null>(null); const frameRef = useRef(null); + const resizeSettleTimerRef = useRef | null>(null); + const resizeReleaseFrameRef = useRef(null); + const resizePointerActiveRef = useRef(false); + const pendingViewportRef = useRef({ width: 0, height: 0 }); const forcePaintRef = useRef(true); const canvasWindowRef = useRef({ top: 0, height: 0 }); const [viewport, setViewport] = useState({ width: 0, height: 0 }); @@ -142,7 +147,7 @@ export function DiffSurface(props: DiffSurfaceProps) { frameRef.current = null; const canvas = canvasRef.current; const currentModel = modelRef.current; - if (!canvas || !currentModel || viewport.width <= 0 || viewport.height <= 0) return; + if (!canvas || !currentModel || currentModel.viewportWidth <= 0 || viewport.height <= 0) return; const desiredHeight = Math.min( Math.max(currentModel.height, viewport.height), viewport.height * 3, @@ -168,7 +173,7 @@ export function DiffSurface(props: DiffSurfaceProps) { : currentWindow.top; const canvasTop = Math.round(requestedCanvasTop * ratio) / ratio; canvasWindowRef.current = { top: canvasTop, height: canvasHeight }; - const pixelWidth = Math.ceil(viewport.width * ratio); + const pixelWidth = Math.ceil(currentModel.viewportWidth * ratio); const pixelHeight = Math.ceil(canvasHeight * ratio); const resized = canvas.width !== pixelWidth || canvas.height !== pixelHeight; if (canvas.width !== pixelWidth) canvas.width = pixelWidth; @@ -198,7 +203,7 @@ export function DiffSurface(props: DiffSurfaceProps) { typography: loadedTypography, measureText: measurement, scrollTop: canvasTop, - viewportWidth: viewport.width, + viewportWidth: currentModel.viewportWidth, viewportHeight: canvasHeight, horizontalOffsets: horizontalOffsetsRef.current, selection: selectionRef.current, @@ -206,7 +211,7 @@ export function DiffSurface(props: DiffSurfaceProps) { paintTop, paintHeight, }); - }, [loadedTypography, measurement, props.palette, viewport.height, viewport.width]); + }, [loadedTypography, measurement, props.palette, viewport.height]); const schedulePaint = useCallback( (force = true) => { if (force) forcePaintRef.current = true; @@ -218,12 +223,56 @@ export function DiffSurface(props: DiffSurfaceProps) { useLayoutEffect(() => { const root = rootRef.current; if (!root) return; + const commitPendingViewport = () => setViewport(pendingViewportRef.current); + const clearResizeSettleTimer = () => { + if (resizeSettleTimerRef.current === null) return; + clearTimeout(resizeSettleTimerRef.current); + resizeSettleTimerRef.current = null; + }; + const handlePointerDown = () => { + resizePointerActiveRef.current = true; + clearResizeSettleTimer(); + }; + const handlePointerEnd = () => { + if (!resizePointerActiveRef.current) return; + resizePointerActiveRef.current = false; + if (resizeReleaseFrameRef.current !== null) { + cancelAnimationFrame(resizeReleaseFrameRef.current); + } + resizeReleaseFrameRef.current = requestAnimationFrame(() => { + resizeReleaseFrameRef.current = null; + commitPendingViewport(); + }); + }; const observer = new ResizeObserver(([entry]) => { if (!entry) return; - setViewport({ width: entry.contentRect.width, height: entry.contentRect.height }); + const nextViewport = { width: entry.contentRect.width, height: entry.contentRect.height }; + pendingViewportRef.current = nextViewport; + setViewport((currentViewport) => + currentViewport.width === 0 || currentViewport.height === 0 + ? nextViewport + : currentViewport, + ); + if (resizePointerActiveRef.current) return; + clearResizeSettleTimer(); + resizeSettleTimerRef.current = setTimeout(() => { + resizeSettleTimerRef.current = null; + commitPendingViewport(); + }, RESIZE_SETTLE_DELAY_MS); }); observer.observe(root); - return () => observer.disconnect(); + window.addEventListener("pointerdown", handlePointerDown, { capture: true }); + window.addEventListener("pointerup", handlePointerEnd, { capture: true }); + window.addEventListener("pointercancel", handlePointerEnd, { capture: true }); + return () => { + observer.disconnect(); + clearResizeSettleTimer(); + if (resizeReleaseFrameRef.current !== null) + cancelAnimationFrame(resizeReleaseFrameRef.current); + window.removeEventListener("pointerdown", handlePointerDown, { capture: true }); + window.removeEventListener("pointerup", handlePointerEnd, { capture: true }); + window.removeEventListener("pointercancel", handlePointerEnd, { capture: true }); + }; }, []); useEffect(() => { let cancelled = false; @@ -255,7 +304,7 @@ export function DiffSurface(props: DiffSurfaceProps) { props.files.map((file) => file.path), ); }, [props.files]); - useEffect(schedulePaint, [model, schedulePaint]); + useLayoutEffect(schedulePaint, [model, schedulePaint]); useEffect( () => () => { if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); @@ -479,10 +528,11 @@ export function DiffSurface(props: DiffSurfaceProps) { const canvasStyle = useMemo( () => ({ ...CANVAS_STYLE, + width: model.viewportWidth, fontFamily: (loadedTypography ?? desiredTypography).family, fontSize: (loadedTypography ?? desiredTypography).size, }), - [desiredTypography, loadedTypography], + [desiredTypography, loadedTypography, model.viewportWidth], ); return ( @@ -730,9 +780,7 @@ const CANVAS_STYLE: React.CSSProperties = { position: "absolute", top: 0, left: 0, - right: 0, zIndex: 1, - width: "100%", pointerEvents: "none", }; const AFFORDANCE_STYLE: ViewStyle = { diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index 7d5fa33cc..de59f700e 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -19,8 +19,9 @@ import { ChevronDown, Columns2, FolderTree, + ListChevronsDownUp, + ListChevronsUpDown, Maximize2, - Minimize2, Pilcrow, RotateCw, WrapText, @@ -158,8 +159,9 @@ const ThemedColumns2 = withUnistyles(Columns2); const ThemedPilcrow = withUnistyles(Pilcrow); const ThemedWrapText = withUnistyles(WrapText); const ThemedFolderTree = withUnistyles(FolderTree); +const ThemedListChevronsDownUp = withUnistyles(ListChevronsDownUp); +const ThemedListChevronsUpDown = withUnistyles(ListChevronsUpDown); const ThemedMaximize2 = withUnistyles(Maximize2); -const ThemedMinimize2 = withUnistyles(Minimize2); const ThemedChevronDown = withUnistyles(ChevronDown); const DIFF_OPTIONS_WHITESPACE_ICON = ( @@ -395,9 +397,15 @@ function CollapseAllToggle({ onPress={allCollapsed ? onExpandAll : onCollapseAll} > {allCollapsed ? ( - + ) : ( - + )}