Conversation
🎩 PreviewA preview build has been created at: |
237dc69 to
b9fa85e
Compare
Duplicate, subtly different copy-failure string — 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 So the PR simultaneously creates the shared constant and lights up a hand-rolled near-duplicate of it: They differ only by "the", so users now get two wordings for one failure. Suggest importing |
b9fa85e to
f674c1e
Compare
Re: the duplicate copy-failure string — fixed in f674c1e.
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). |
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-nodesJSON envelope to the system clipboard and paste read it back — but the Cmd+V keydown shortcut calledpreventDefault(), which suppressed the nativepasteevent entirely. That left paste dependent onnavigator.clipboard.readText(), which is gated behind theclipboard-readpermission in Chrome and is not freely available to pages in Firefox. Every failure was swallowed by a barecatch {}, 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
pasteevent.readPasteEventClipboardInfopulls the envelope out ofClipboardEvent.clipboardData, which needs no permission and works in every browser without a prompt.useClipboardShortcutsnow listens forpasteonwindowand pastes at the canvas centre as before. It bails on:isEditableTarget— inputs, textareas, contenteditable, Monaco), so text still pastes normally, andisDialogOpen), 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 onbodythe 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 mountsContentwhile open.The Cmd+V keydown shortcut stays registered so it remains discoverable in the shortcut list, but its action returns
false— the mechanismShortcutDefinitionalready documents for "let the native event propagate" — so the listener skipspreventDefaultand the browser goes on to firepaste.ClipboardStore.pastetreats 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 toreadSystemClipboardInfo()if an event arrives withoutclipboardData.2. Clipboard failures surface, with one convention in both directions.
writeToSystemClipboardrejects instead of swallowing, andpasterejects 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
catchinDashboardComponentsV2View.handleCopyToPipelinethat could never fire before, becausewriteToSystemClipboardswallowed 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:
SystemClipboardInfotype and oneclassify()helper serve both readers. Previously there were two near-identicalreadText+ parse functions with different return shapes.collectNodeSnapshotsis shared byClipboardStoreandcopyNodesToClipboard, which each ran the same manifest-snapshot loop plussnapshotInternalBindings. ThecloneHandlerfallback is preserved (load-bearing for FlexNode, a no-op for RunView manifests, which define nocloneHandler).isClipboardEnvelopenow validates thatsnapshotsandbindingsare arrays, not just that_typematches. Without that,{"_type":"tangle-pipeline-nodes","snapshots":"xx"}on the system clipboard passed the guard and threw incomputeSnapshotBounds. That mattered much more onceclipboardDatamade every ⌘V over the canvas feed real clipboard text into this path.Not in scope
Ctrl+C/Ctrl+Vare still unbound. The clipboard shortcuts register againstCMDALT, whichkeys.tsproduces only from Meta or Alt —Controlmaps to a separateCTRLconstant andmatchesPressedrequires an exact set match. On macOS that is fine (⌘C/⌘V). On Windows/Linux onlyAlt+C/Alt+Vwork, whileShortcutBadgerendersCMDALTas 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
Checklist
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):
Regressions to check:
Failure feedback (needs devtools):
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 (
clipboardDatapresent, targetBODY); 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 toshortcutUtils.test.tsfor 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-arraysnapshots, missingsnapshots) being rejected at the boundary rather than crashing downstream.clipboardStore.test.tsmocks@/routes/v2/pages/Editor/nodes— the real node registry transitively imports the router, so the store cannot be unit-tested against it.