Skip to content

fix: Copy + Paste Across Tabs - #2705

Open
camielvs wants to merge 1 commit into
masterfrom
fix-copy-paste-again
Open

camielvs wants to merge 1 commit into
masterfrom
fix-copy-paste-again

Conversation

@camielvs

@camielvs camielvs commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Description

Copying nodes in one tab and pasting them in another silently did nothing. Cross-instance transfer was already the intent — copy wrote a tangle-pipeline-nodes JSON envelope to the system clipboard and paste read it back — but the Cmd+V keydown shortcut called preventDefault(), which suppressed the native paste event entirely. That left paste dependent on navigator.clipboard.readText(), which is gated behind the clipboard-read permission in Chrome and is not freely available to pages in Firefox. Every failure was swallowed by a bare catch {}, and paste then fell back to the in-memory store, which is empty in a fresh tab. Result: nothing pasted, no feedback.

Three changes:

1. Paste reads from the native paste event.

readPasteEventClipboardInfo pulls the envelope out of ClipboardEvent.clipboardData, which needs no permission and works in every browser without a prompt. useClipboardShortcuts now listens for paste on window and pastes at the canvas centre as before. It bails on:

  • editable targets (isEditableTarget — inputs, textareas, contenteditable, Monaco), so text still pastes normally, and
  • any open dialog (isDialogOpen), so a dialog owns the paste rather than nodes landing on the canvas behind it. This checks the document rather than the event target, because with a dialog open and focus on body the target alone doesn't tell you a dialog is up. Covers Dialog, AlertDialog and Sheet — all three build on @radix-ui/react-dialog, and Radix only mounts Content while open.

The Cmd+V keydown shortcut stays registered so it remains discoverable in the shortcut list, but its action returns false — the mechanism ShortcutDefinition already documents for "let the native event propagate" — so the listener skips preventDefault and the browser goes on to fire paste.

ClipboardStore.paste treats a paste-event read as authoritative and skips the async read when it has one, so the permission prompt is gone on the normal path. It still falls back to readSystemClipboardInfo() if an event arrives without clipboardData.

2. Clipboard failures surface, with one convention in both directions.

writeToSystemClipboard rejects instead of swallowing, and paste rejects when the clipboard is unreadable and nothing is staged in memory. Copy and paste therefore fail the same way, and each call site needs a single .catch(). All four copy call sites (editor shortcut and toolbar, run view shortcut and toolbar) and the paste handler report failures via toast.

A readable clipboard holding no nodes resolves silently — pasting ordinary text over the canvas shouldn't nag.

This also revives a catch in DashboardComponentsV2View.handleCopyToPipeline that could never fire before, because writeToSystemClipboard swallowed the error internally — "Copy to pipeline" reported success unconditionally, even when the clipboard write was refused. That handler now shares the same message constant rather than hand-rolling its own wording.

3. Shared clipboard helpers.

Three pieces of duplication removed while the surrounding code was being reworked:

  • One SystemClipboardInfo type and one classify() helper serve both readers. Previously there were two near-identical readText + parse functions with different return shapes.
  • collectNodeSnapshots is shared by ClipboardStore and copyNodesToClipboard, which each ran the same manifest-snapshot loop plus snapshotInternalBindings. The cloneHandler fallback is preserved (load-bearing for FlexNode, a no-op for RunView manifests, which define no cloneHandler).
  • isClipboardEnvelope now validates that snapshots and bindings are arrays, not just that _type matches. Without that, {"_type":"tangle-pipeline-nodes","snapshots":"xx"} on the system clipboard passed the guard and threw in computeSnapshotBounds. That mattered much more once clipboardData made every ⌘V over the canvas feed real clipboard text into this path.

Not in scope

Ctrl+C/Ctrl+V are still unbound. The clipboard shortcuts register against CMDALT, which keys.ts produces only from Meta or Alt — Control maps to a separate CTRL constant and matchesPressed requires an exact set match. On macOS that is fine (⌘C/⌘V). On Windows/Linux only Alt+C/Alt+V work, while ShortcutBadge renders CMDALT as the literal string "Ctrl" — so the UI advertises a binding that does not exist. Worth a separate fix; it is a display/binding mismatch rather than a clipboard bug.

Related Issue and Pull requests

None.

Type of Change

  • Bug fix
  • New feature
  • Improvement
  • Cleanup/Refactor
  • Breaking change
  • Documentation update

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Unit suite: 2490 passing across 232 files. Typecheck, lint and format clean. The Playwright E2E suite was not run — it has no coverage of copy/paste.

Screenshots (if applicable)

No visual change beyond the two error toasts, whose copy is in clipboardMessages.ts:

  • Couldn't copy to the clipboard. Check browser permissions and try again.
  • Couldn't read the clipboard. Check browser permissions and try again.

Test Instructions

Cross-tab paste (the actual bug):

  1. Open the V2 editor on a pipeline with at least one task.
  2. Select one or more nodes and press ⌘C.
  3. Open a second tab (or a second instance) on a different pipeline in the V2 editor.
  4. Click the canvas and press ⌘V. The nodes, and any connections purely between them, should appear centred in the viewport. No clipboard permission prompt should appear.
  5. Press ⌘V again — each paste cascades by 50px rather than stacking.

Regressions to check:

  1. Paste text into the component search field, a task argument, and the Monaco editor — all should behave natively, with no node created.
  2. Open any dialog over the canvas (e.g. "Create subgraph"), then press ⌘V without focusing a field. Nothing should paste onto the canvas behind it.
  3. ⌘C with a text selection on the page should still copy that text rather than the selected nodes.
  4. Same-tab copy/paste and ⌘D duplicate should be unchanged.
  5. Pasting unrelated text over the canvas should do nothing and show no toast.

Failure feedback (needs devtools):

  1. In the console, run:
    Object.defineProperty(navigator, "clipboard", {
      value: {
        writeText: () => Promise.reject(new Error("NotAllowed")),
        readText: () => Promise.reject(new Error("NotAllowed")),
      },
      configurable: true,
    });
  2. Select a node and press ⌘C → copy-failure toast.
  3. Reload, re-apply the override, then run
    document.body.dispatchEvent(new ClipboardEvent("paste", {bubbles: true, cancelable: true}))
    → read-failure toast. (A synthetic event carries no clipboardData, so this is the one path that still falls through to the async read.)

Additional Comments

Verified by driving the real app in Chromium against the dev server: an envelope placed on the system clipboard pasted into the V2 canvas as a task node, with the native paste event reaching the listener (clipboardData present, target BODY); pasting into the component search field still inserted text with no node created; and both toasts appeared under a forced-failure clipboard.

Tests: 13 for the envelope read/write paths (clipboardEnvelope.test.ts), 8 for the store (clipboardStore.test.ts), and 3 added to shortcutUtils.test.ts for the dialog guard. Coverage includes an envelope from another tab short-circuiting the async read, a denied read rejecting, the paste offset not advancing when nothing is pasted, and malformed envelopes (non-array snapshots, missing snapshots) being rejected at the boundary rather than crashing downstream.

clipboardStore.test.ts mocks @/routes/v2/pages/Editor/nodes — the real node registry transitively imports the router, so the store cannot be unit-tested against it.

@camielvs
camielvs requested a review from a team as a code owner September 5, 2026 01:19
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

🎩 Preview

A preview build has been created at: fix-copy-paste-again/f674c1e

@camielvs
camielvs force-pushed the fix-copy-paste-again branch from 237dc69 to b9fa85e Compare September 10, 2026 19:02
Comment thread src/routes/v2/pages/Editor/components/FlowCanvas/hooks/useClipboardShortcuts.ts Outdated
Comment thread src/routes/v2/pages/Editor/store/clipboardStore.ts Outdated
Comment thread src/routes/v2/pages/Editor/components/FlowCanvas/hooks/useClipboardShortcuts.ts Outdated
@camielvs

Copy link
Copy Markdown
Collaborator Author

🤖 This is an AI-generated code review comment.

Duplicate, subtly different copy-failure stringsrc/routes/Dashboard/DashboardComponentsV2View.tsx:1395

Posting this at top level because the file isn't part of this PR's diff, so it can't take an inline comment.

This PR introduces clipboardMessages.ts for exactly this string, and — as the description notes — makes this catch reachable for the first time by removing the internal swallow in writeToSystemClipboard. The chain checks out: handleCopyToPipelinecopyComponentReferenceToClipboardwriteToSystemClipboard.

So the PR simultaneously creates the shared constant and lights up a hand-rolled near-duplicate of it:

"Couldn't copy to clipboard. Check browser permissions and try again."      // line 1395
"Couldn't copy to the clipboard. Check browser permissions and try again."  // CLIPBOARD_COPY_FAILED_MESSAGE

They differ only by "the", so users now get two wordings for one failure. Suggest importing CLIPBOARD_COPY_FAILED_MESSAGE there.

@camielvs
camielvs force-pushed the fix-copy-paste-again branch from b9fa85e to f674c1e Compare September 10, 2026 19:36
@camielvs

Copy link
Copy Markdown
Collaborator Author

🤖 AI-generated follow-up.

Re: the duplicate copy-failure string — fixed in f674c1e.

DashboardComponentsV2View.tsx now imports CLIPBOARD_COPY_FAILED_MESSAGE instead of hand-rolling its own wording, so the newly-reachable catch and the rest of the clipboard paths show one message.

All four review findings are now addressed; the three inline threads have been replied to and resolved. Full suite green: 232 files, 2490 tests (5 new — two for the tightened envelope guard, three for the dialog guard).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant