Restore copy actions in web diffs (#4229)

* fix(app): restore diff copy actions on web The canvas-backed diff owns selection without creating a DOM range, so browser context menus cannot expose copy actions themselves. Route keyboard and context-menu actions through the renderer's canonical source selection instead. * fix(app): harden web diff copy actions * fix(app): default split header selection to new pane * fix(app): isolate nested diff context menus * fix(app): select available split diff pane
2026-10-28 20:22:45 +00:00 · 2026-09-02 20:25:55 +02:00
parent 53d824a320
commit 2d8ff6f1cf
13 changed files with 387 additions and 21 deletions
@@ -4,6 +4,8 @@ import {
  characterOffsetAtPoint,
  hitTestDiffDocument,
  orderedSelection,
+  selectAllSource,
+  selectCellSource,
  selectedSourceText,
 } from "./hit-testing";
 import { buildDiffDocumentModel } from "./model";
@@ -104,6 +106,64 @@ describe("diff hit testing", () => {
      }),
    ).toBeNull();
  });
+
+  it("selects one source line for context-menu copy", () => {
+    const model = buildModel("copy this", { wrapLines: false });
+    const cell = changedCell(model);
+
+    expect(selectedSourceText(model, selectCellSource(model, position(model, cell, 4)))).toBe(
+      "copy this",
+    );
+  });
+
+  it("selects all diff source in document order", () => {
+    const model = buildModel("first", { wrapLines: false });
+
+    expect(selectedSourceText(model, selectAllSource(model)!)).toBe("@@ -1,0 +1,2 @@\nfirst\ntail");
+  });
+
+  it("selects all source from one split side without interleaving panes", () => {
+    const model = buildModel("unused", { files: [splitFile()], layout: "split" });
+    const changedRow = model.rows.find(
+      (row) => row.kind === "line" && row.cells.length === 2 && row.cells.every(Boolean),
+    );
+    expect(changedRow?.kind).toBe("line");
+
+    const left = changedRow!.kind === "line" ? changedRow!.cells[0]! : null;
+    const right = changedRow!.kind === "line" ? changedRow!.cells[1]! : null;
+    expect(selectedSourceText(model, selectAllSource(model, position(model, left!, 0))!)).toBe(
+      "@@ -1,2 +1,2 @@\nold-one\nold-two",
+    );
+    expect(selectedSourceText(model, selectAllSource(model, position(model, right!, 0))!)).toBe(
+      "new-one\nnew-two",
+    );
+  });
+
+  it("defaults split hunk-header select-all to the new pane", () => {
+    const model = buildModel("unused", { files: [splitFile()], layout: "split" });
+    const header = model.rows
+      .filter((row) => row.kind === "line")
+      .flatMap((row) => row.cells)
+      .find((cell) => cell?.type === "header");
+    expect(header).toBeDefined();
+
+    expect(selectedSourceText(model, selectAllSource(model, position(model, header!, 0))!)).toBe(
+      "new-one\nnew-two",
+    );
+  });
+
+  it("falls back to the old pane for a deletion-only split hunk", () => {
+    const model = buildModel("unused", { files: [deletedSplitFile()], layout: "split" });
+    const header = model.rows
+      .filter((row) => row.kind === "line")
+      .flatMap((row) => row.cells)
+      .find((cell) => cell?.type === "header");
+    expect(header).toBeDefined();
+
+    expect(selectedSourceText(model, selectAllSource(model, position(model, header!, 0))!)).toBe(
+      "@@ -1,2 +1,0 @@\nold-one\nold-two",
+    );
+  });
 });

 function buildModel(changedContent: string, overrides: Partial<BuildDiffDocumentModelInput> = {}) {
@@ -212,6 +272,29 @@ function splitFile(): ParsedDiffFile {
  };
 }

+function deletedSplitFile(): ParsedDiffFile {
+  return {
+    path: "src/deleted.ts",
+    isNew: false,
+    isDeleted: true,
+    additions: 0,
+    deletions: 2,
+    hunks: [
+      {
+        oldStart: 1,
+        oldCount: 2,
+        newStart: 1,
+        newCount: 0,
+        lines: [
+          { type: "header", content: "@@ -1,2 +1,0 @@" },
+          { type: "remove", content: "old-one" },
+          { type: "remove", content: "old-two" },
+        ],
+      },
+    ],
+  };
+}
+
 function reviewActionsForFirstAddition(): NonNullable<
  BuildDiffDocumentModelInput["reviewActions"]
 > {
@@ -105,6 +105,64 @@ export function selectedSourceText(model: DiffDocumentModel, selection: DiffSele
    .join("\n");
 }

+export function selectCellSource(
+  model: DiffDocumentModel,
+  position: DiffCharacterPosition,
+): DiffSelection {
+  const row = model.rows[position.rowIndex];
+  const cell = row?.kind === "line" ? row.cells[position.cellIndex] : null;
+  const end = cell?.content.length ?? position.sourceOffset;
+  return {
+    anchor: { ...position, sourceOffset: 0 },
+    focus: { ...position, sourceOffset: end },
+  };
+}
+
+export function selectAllSource(
+  model: DiffDocumentModel,
+  preferredPosition?: DiffCharacterPosition,
+): DiffSelection | null {
+  let first: DiffCharacterPosition | null = null;
+  let last: DiffCharacterPosition | null = null;
+  const preferredRow = preferredPosition ? model.rows[preferredPosition.rowIndex] : null;
+  const preferredCell =
+    preferredPosition && preferredRow?.kind === "line"
+      ? preferredRow.cells[preferredPosition.cellIndex]
+      : null;
+  let splitCellIndex: number | null = null;
+  if (model.layout === "split") {
+    const hasNewSource = model.rows.some(
+      (row) => row.kind === "line" && row.cells[1] && row.cells[1].type !== "header",
+    );
+    splitCellIndex = hasNewSource ? 1 : 0;
+    if (preferredPosition && preferredCell && preferredCell.type !== "header") {
+      splitCellIndex = preferredPosition.cellIndex;
+    }
+  }
+  for (const row of model.rows) {
+    if (row.kind !== "line") continue;
+    row.cells.forEach((cell, cellIndex) => {
+      if (!cell || (splitCellIndex !== null && cellIndex !== splitCellIndex)) return;
+      const side = cell.reviewTarget?.side ?? (cellIndex === 0 ? "old" : "new");
+      first ??= {
+        fileIndex: row.fileIndex,
+        rowIndex: row.index,
+        cellIndex,
+        side,
+        sourceOffset: 0,
+      };
+      last = {
+        fileIndex: row.fileIndex,
+        rowIndex: row.index,
+        cellIndex,
+        side,
+        sourceOffset: cell.content.length,
+      };
+    });
+  }
+  return first && last ? { anchor: first, focus: last } : null;
+}
+
 function comparePosition(left: DiffCharacterPosition, right: DiffCharacterPosition): number {
  return (
    left.rowIndex - right.rowIndex ||
@@ -2,7 +2,16 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } fr
 import { useTranslation } from "react-i18next";
 import type { ViewStyle } from "react-native";
 import { DomOverlayScrollbar } from "@/components/ui/overlay-scrollbar/dom-overlay-scrollbar";
+import {
+  ContextMenu,
+  ContextMenuContent,
+  ContextMenuItem,
+  ContextMenuSeparator,
+  ContextMenuTrigger,
+} from "@/components/ui/context-menu";
+import { useToast } from "@/contexts/toast-context";
 import { InlineReviewAddButton, InlineReviewThread } from "@/review";
+import { copyToClipboard } from "@/utils/copy-to-clipboard";
 import type { ReviewableDiffTarget } from "@/utils/diff-layout";
 import { DocumentFileHeader } from "./document-file-header";
 import {
@@ -11,7 +20,12 @@ import {
  diffMaterializationWindow,
  resolveVisibleFileSections,
 } from "./header-layout";
-import { hitTestDiffDocument, selectedSourceText } from "./hit-testing";
+import {
+  hitTestDiffDocument,
+  selectAllSource,
+  selectCellSource,
+  selectedSourceText,
+} from "./hit-testing";
 import { retainHorizontalOffsetMapForPaths } from "./horizontal-offsets";
 import { HorizontalScroll } from "./horizontal-scroll.web";
 import { buildDiffDocumentModel, FILE_HEADER_HEIGHT, resolveRelayoutScrollTop } from "./model";
@@ -33,6 +47,7 @@ const RESIZE_SETTLE_DELAY_MS = 120;

 export function DiffSurface(props: DiffSurfaceProps) {
  const { t } = useTranslation();
+  const toast = useToast();
  const workspaceCache = useDiffDocumentWorkspaceCache();
  const rootRef = useRef<HTMLDivElement>(null);
  const canvasRef = useRef<HTMLCanvasElement>(null);
@@ -75,6 +90,8 @@ export function DiffSurface(props: DiffSurfaceProps) {
  const [interactionFiles, setInteractionFiles] = useState<
    ReturnType<typeof buildDiffDocumentModel>["files"]
  >([]);
+  const [hasSelection, setHasSelection] = useState(false);
+  const [contextHit, setContextHit] = useState<Extract<DiffHit, { kind: "cell" }> | null>(null);
  const [hoveredAffordance, setHoveredAffordance] = useState<{
    hit: Extract<DiffHit, { kind: "cell" }>;
    left: number;
@@ -488,13 +505,13 @@ export function DiffSurface(props: DiffSurfaceProps) {
    },
    [schedulePaint],
  );
-  const pointHit = useCallback((event: React.PointerEvent<HTMLDivElement>) => {
+  const pointHitAt = useCallback((clientX: number, clientY: number) => {
    const currentModel = modelRef.current;
    const root = rootRef.current;
    if (!currentModel || !root) return null;
    const bounds = root.getBoundingClientRect();
-    const x = event.clientX - bounds.left;
-    const documentY = event.clientY - bounds.top + scrollTopRef.current;
+    const x = clientX - bounds.left;
+    const documentY = clientY - bounds.top + scrollTopRef.current;
    const file = currentModel.files.find(
      (entry) => entry.top <= documentY && documentY < entry.bottom,
    );
@@ -505,6 +522,27 @@ export function DiffSurface(props: DiffSurfaceProps) {
      horizontalOffset: file ? (horizontalOffsetsRef.current.get(file.path) ?? 0) : 0,
    });
  }, []);
+  const pointHit = useCallback(
+    (event: React.PointerEvent<HTMLDivElement>) => pointHitAt(event.clientX, event.clientY),
+    [pointHitAt],
+  );
+  const setSelection = useCallback(
+    (selection: DiffSelection | null) => {
+      selectionRef.current = selection;
+      const currentModel = modelRef.current;
+      setHasSelection(
+        Boolean(
+          selection && currentModel && selectedSourceText(currentModel, selection).length > 0,
+        ),
+      );
+      schedulePaint();
+    },
+    [schedulePaint],
+  );
+  useEffect(() => {
+    dragRef.current = null;
+    setSelection(null);
+  }, [props.collapsedFilePaths, props.displayPreferences.layout, props.files, setSelection]);
  const pointerDown = useCallback(
    (event: React.PointerEvent<HTMLDivElement>) => {
      if (event.button !== 0) return;
@@ -518,8 +556,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
        return;
      const dismissSelectionOnClick = selectionRef.current !== null;
      if (dismissSelectionOnClick) {
-        selectionRef.current = null;
-        schedulePaint();
+        setSelection(null);
      }
      const hit = pointHit(event);
      if (hit?.kind !== "cell") return;
@@ -530,12 +567,11 @@ export function DiffSurface(props: DiffSurfaceProps) {
        moved: false,
        dismissSelectionOnClick,
      };
-      selectionRef.current = { anchor: hit.position, focus: hit.position };
+      setSelection({ anchor: hit.position, focus: hit.position });
      event.currentTarget.focus();
      event.currentTarget.setPointerCapture(event.pointerId);
-      schedulePaint();
    },
-    [pointHit, schedulePaint],
+    [pointHit, setSelection],
  );
  const updateActiveHeader = useCallback(
    (target: EventTarget | null) => {
@@ -589,10 +625,9 @@ export function DiffSurface(props: DiffSurfaceProps) {
        setHoveredAffordance(null);
      }
      if (!drag || hit?.kind !== "cell") return;
-      selectionRef.current = { anchor: drag.anchor, focus: hit.position };
-      schedulePaint();
+      setSelection({ anchor: drag.anchor, focus: hit.position });
    },
-    [pointHit, schedulePaint, updateActiveHeader, viewport.width],
+    [pointHit, setSelection, updateActiveHeader, viewport.width],
  );
  const pointerUp = useCallback(
    (event: React.PointerEvent<HTMLDivElement>) => {
@@ -612,8 +647,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
          })
        : false;
      if (drag && !moved && drag.dismissSelectionOnClick) {
-        selectionRef.current = null;
-        schedulePaint();
+        setSelection(null);
        return;
      }
      if (
@@ -624,12 +658,11 @@ export function DiffSurface(props: DiffSurfaceProps) {
        hit.target &&
        reviewActions
      ) {
-        selectionRef.current = null;
+        setSelection(null);
        reviewActions.onStartComment(hit.target);
-        schedulePaint();
      }
    },
-    [pointHit, reviewActions, schedulePaint],
+    [pointHit, reviewActions, setSelection],
  );
  const cancelPointer = useCallback(() => {
    dragRef.current = null;
@@ -643,6 +676,61 @@ export function DiffSurface(props: DiffSurfaceProps) {
    },
    [model],
  );
+  const prepareContextMenu = useCallback(
+    (event: React.MouseEvent<HTMLDivElement>) => {
+      const hit = pointHitAt(event.clientX, event.clientY);
+      setContextHit(hit?.kind === "cell" ? hit : null);
+    },
+    [pointHitAt],
+  );
+  const writeSourceText = useCallback(
+    (text: string) => {
+      void copyToClipboard(text)
+        .then(() => toast.copied(t("common.states.copied")))
+        .catch(() => toast.error(t("common.errors.unableToCopy")));
+    },
+    [t, toast],
+  );
+  const copySelectedSource = useCallback(() => {
+    const currentModel = modelRef.current;
+    const selection = selectionRef.current;
+    if (currentModel && selection) writeSourceText(selectedSourceText(currentModel, selection));
+  }, [writeSourceText]);
+  const copyContextLine = useCallback(() => {
+    const currentModel = modelRef.current;
+    if (!currentModel || !contextHit) return;
+    const selection = selectCellSource(currentModel, contextHit.position);
+    writeSourceText(selectedSourceText(currentModel, selection));
+  }, [contextHit, writeSourceText]);
+  const selectAll = useCallback(() => {
+    const currentModel = modelRef.current;
+    if (currentModel) {
+      setSelection(
+        selectAllSource(currentModel, contextHit?.position ?? selectionRef.current?.focus),
+      );
+    }
+  }, [contextHit?.position, setSelection]);
+  const keyDown = useCallback(
+    (event: React.KeyboardEvent<HTMLDivElement>) => {
+      const target = event.target;
+      if (
+        target instanceof Element &&
+        target.closest('input, textarea, [contenteditable="true"]')
+      ) {
+        return;
+      }
+      if ((event.metaKey || event.ctrlKey) && event.key.toLowerCase() === "a") {
+        event.preventDefault();
+        selectAll();
+        return;
+      }
+      if (event.key === "Escape" && selectionRef.current) {
+        event.preventDefault();
+        setSelection(null);
+      }
+    },
+    [selectAll, setSelection],
+  );
  const rootStyle = useMemo<React.CSSProperties>(
    () => ({ ...ROOT_STYLE, background: props.palette.surface }),
    [props.palette.surface],
@@ -672,7 +760,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
    [desiredTypography, loadedTypography],
  );

-  return (
+  const surface = (
    <div data-testid="git-diff-canvas-root" ref={rootRef} style={rootStyle}>
      <div
        ref={scrollRef}
@@ -685,7 +773,9 @@ export function DiffSurface(props: DiffSurfaceProps) {
        onPointerCancel={cancelPointer}
        onLostPointerCapture={cancelPointer}
        onPointerLeave={pointerLeave}
+        onContextMenu={prepareContextMenu}
        onCopy={copy}
+        onKeyDown={keyDown}
        style={SCROLL_STYLE}
      >
        <div style={contentStyle} onMouseDown={preventDocumentMouseSelection}>
@@ -752,6 +842,34 @@ export function DiffSurface(props: DiffSurfaceProps) {
      ) : null}
    </div>
  );
+
+  return (
+    <ContextMenu>
+      <ContextMenuTrigger contextOnly style={CONTEXT_TRIGGER_STYLE}>
+        {surface}
+      </ContextMenuTrigger>
+      <ContextMenuContent align="start" minWidth={180} testID="diff-source-context-menu">
+        <ContextMenuItem
+          disabled={!hasSelection}
+          onSelect={copySelectedSource}
+          testID="diff-source-copy-selection"
+        >
+          {t("common.actions.copy")}
+        </ContextMenuItem>
+        <ContextMenuItem
+          disabled={!contextHit}
+          onSelect={copyContextLine}
+          testID="diff-source-copy-line"
+        >
+          {t("common.actions.copyLine")}
+        </ContextMenuItem>
+        <ContextMenuSeparator />
+        <ContextMenuItem onSelect={selectAll} testID="diff-source-select-all">
+          {t("common.actions.selectAll")}
+        </ContextMenuItem>
+      </ContextMenuContent>
+    </ContextMenu>
+  );
 }

 function shiftCanvasPixels(input: {
@@ -820,7 +938,7 @@ function WebFileHeaderSection({
    [file.bottom, file.top],
  );
  return (
-    <div style={style}>
+    <div style={style} onContextMenu={stopContextMenuPropagation}>
      <div
        data-diff-header="true"
        data-diff-header-path={file.path}
@@ -864,7 +982,7 @@ function WebReviewThread({
    [height, left, top, width],
  );
  return (
-    <div style={style} data-diff-review="true">
+    <div style={style} data-diff-review="true" onContextMenu={stopContextMenuPropagation}>
      <InlineReviewThread
        reviewTarget={target}
        reviewActions={actions}
@@ -876,12 +994,17 @@ function WebReviewThread({
  );
 }

+function stopContextMenuPropagation(event: React.MouseEvent): void {
+  event.stopPropagation();
+}
+
 const ROOT_STYLE: React.CSSProperties = {
  position: "relative",
  flex: 1,
  minHeight: 0,
  overflow: "hidden",
 };
+const CONTEXT_TRIGGER_STYLE: ViewStyle = { flex: 1, minHeight: 0 };
 const SCROLL_STYLE: React.CSSProperties = {
  position: "absolute",
  inset: 0,