[ENG-2149] Support dropping multiple images onto the canvas

[mattakamatsu]

Closes ENG-2149.

What was wrong

tldraw splits a file drop across two layers. A content handler for "files" owns iterating the dropped files, enforcing size and type limits, and placing the shapes. An asset store owns turning one file into one stored asset, meaning where the bytes go.

The canvas overrode the content handler, and that override read content.files[0] and nothing else. Every file after the first was discarded. It also ignored content.point, so the one image it did create landed at the centre of the viewport instead of where you dropped it.

The override existed for a real reason: canvas media has to go to Roam's file store, not be inlined as base64 into the page's block props. But that is an asset-store concern implemented one layer too high, and it took multi-file handling down with it.

What changed

The Roam upload moves down to the asset store, and the content-handler override is deleted.

• apps/roam/src/utils/roamCanvasAssetStore.ts is the shared asset store.
• The Cloudflare sync adapter already had this exact asset store, so it now shares the module.
• The local block-props store had no asset store at all, which is why the override was written in the first place. It gets one now, so both canvas paths behave identically.

With the override gone, tldraw's own handler takes back over and the canvas matches tldraw.com.

Where images live: unchanged

Before and after, media is uploaded through window.roamAlphaAPI.file.upload into Roam's Firebase-backed file store at imgs/app/<graph>/<id>.<ext>. The canvas stores only the URL, in the tldraw asset record's props.src. Image bytes never enter block props. Same API call, same destination, same field. Only the layer that makes the call changed.

Existing canvases are unaffected and no migration is involved. Only new uploads take the new code path.

Reviewer notes

Asset IDs now hash file content rather than the upload URL. This is tldraw's default and the one behavioural change worth a second opinion. Identical images now collapse to one asset record. Measured: dropping the same image three times gives 3 uploads, 1 asset record, 3 correctly-rendering shapes, and therefore 2 orphaned files in Roam. Context: the plugin never calls file.delete anywhere, so deleting an image shape already orphaned its upload. Reclaiming unreferenced uploads is separate work and deliberately not in here.

file.upload returns Roam markup, not a URL, and the wrapper depends on file type. ![](url) for images, {{[[video]]: url}} for video, {{[[audio]]: …}}, {{[[pdf]]: …}}, [name](url) otherwise. The parser inherited from the sync adapter stripped the image wrapper specifically, so the first real video dropped crashed the canvas with a schema error. It now extracts the URL from whatever wrapper came back. This was already latent in the sync adapter before this PR; it was simply unreachable, because the deleted handler rejected video before the asset store ever saw it.

A bad src used to take down the whole canvas. It fails inside store.put, which is past the try/catch in tldraw's file handler, so the error boundary caught it and offered "Reset data" for the entire canvas. The asset store now validates before returning, which turns it into an "Upload failed" toast for that one file while the rest of the drop still lands.

Smaller consequences of using tldraw's defaults: accepted image types widen to include apng and avif; failures raise a toast instead of a silent console.error; the toolbar's Media insert button was broken the same way and is fixed by the same change, since its picker already sets multiple = true; and Canvas: Asset Added now fires once per file instead of once per drop, with the same event name and source value.

Test plan

Automated:

• 11 unit tests on the asset store, covering each upload wrapper form and both validation branches.
• Full Roam suite passes (107 tests, 18 files). tsc --noEmit clean, eslint 0 errors, extension builds.

Against real tldraw 2.4.6 in a harness driving genuine drop events:

• Three-file drop: 3 uploads, 3 image shapes tiled in a row, centred on the drop point, all selected.
• Control with the old files[0] handler re-registered: 1 upload, 1 shape. Confirms the deleted code was the cause.
• Real 320×240 mp4 dropped alongside a png: both upload, video wrapper parses, both shapes created at correct dimensions.
• Three files where one upload returns prose instead of a URL: one toast, that file skipped, the other two imported, no error boundary.

In a live Roam graph:

• Multi-image drop confirmed working by @mattakamatsu on plugin-testing-akamatsulab2. The mp4 crash above was found this way and is fixed.

Still worth covering before merge: persistence across a page reload, and the same drop on a sync-mode canvas.

Scope check

• Ran $scope-check against ENG-2149 and the final diff.
• Scope beyond Done When: The ticket has no Done When section, so the boundary was read from its one-line description, "drag multiple images into the canvas, as tldraw.com does". Beyond that: (1) video files now import as video shapes, where they were previously rejected with a silent console.error; (2) accepted image types widen to tldraw's defaults, adding apng and avif; (3) upload failures now surface as toasts.
• Required now: All three fall out of delegating to tldraw's default "files" handler, which is the fix itself rather than an addition to it. Keeping the previous behaviour would mean re-implementing the handler's iteration and placement logic in order to narrow it again, which reintroduces the class of bug this PR removes. Video also forced the {{[[video]]: url}} parse fix and the crash-containment work, both of which are needed regardless of whether video stays.
• Anyone affected or consulted: Yes. Raised with @mattakamatsu on the ticket before implementation was reviewed, and again after live testing surfaced the video crash. Explicitly left open for a product decision: if video should not be accepted, the change is to drop acceptedVideoMimeTypes from the handler config, and dropped videos get a clean "type not allowed" toast.
• Decision: https://linear.app/discourse-graphs/issue/ENG-2149/drag-multiple-images-into-tldraw-canvas (see the two implementation comments) — video accept/reject still pending.

🤖 Generated with Claude Code

[linear-code]

ENG-2149

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
discourse-graph	Skipped		Aug 26, 2026 12:36am

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[sid597]

Why the onUpload callback? Both call sites pass this same function. The repo pattern is to call posthog.capture inline at the action site (QueryDrawer, QueryBuilder, DiscourseNodeSearchMenu all do this), and src/utils already imports posthog in 7 files, so there's no layering rule keeping it out of the asset store. Let's move the capture into createRoamAssetStore's upload and delete this file, the onUpload param, the try/catch guard, and its test. About 30 fewer lines and one less indirection to follow.

[mattakamatsu]

Done in 1b648ad. Checked the claim before changing it: src/utils has posthog in 7 files, and the capture is now inline in createRoamAssetStore's upload path.

The unstated reason for the callback was that I expected posthog-js to break the node-env vitest run. It doesn't — the suite passes with the import in place, so the indirection was defending against nothing. canvasAssetTelemetry.ts, the param, the try/catch and its two tests are gone.

One thing I kept: the helper takes the posthog source as a plain argument, defaulting to "file-drop". That's only so svg pastes keep reporting "svg-paste" — routing them through the shared upload (your other comment) would otherwise silently fold that dimension into "file-drop". If losing it is fine, that argument can go too and the signature drops to (file).

[sid597]

This is the same crash path the PR fixes for file drops. parseRoamUploadResponse falls back to the raw response when it can't find a url, and here that flows straight into createAssets and fails later in store.put, past any try/catch, so the whole canvas goes down with the error boundary. The asset store validates before returning for exactly this reason, and both stores now carry it, so editor.uploadAsset(asset, file) is available in this handler. Let's route this upload through the asset store, or at least reuse its url check. If it's out of scope for this PR, a ticket works.

[mattakamatsu]

Confirmed and fixed in 1b648ad, in scope rather than as a ticket.

You're right about the mechanism, and it's worse than pre-existing: my change to parseRoamUploadResponse added the raw-response fallback that this handler consumes, so this PR made the bad value more reachable rather than less.

Rather than editor.uploadAsset I pulled the upload out as uploadCanvasFileToRoam(file, source) and had both the asset store and this handler call it. Same validation and same telemetry for both, and it avoids constructing a placeholder asset just to satisfy uploadAsset's signature when our store ignores that argument anyway. The handler also catches now and shows an "Upload failed" toast instead of rejecting silently.

Verified against tldraw 2.4.6 by driving the svg-text handler directly: a good paste still creates one image shape and asset; an upload response with no url gives a toast, no shape, and no error boundary.

Separately, this handler is now the last hand-rolled upload path left. tldraw's own svg-text default would go through getAssetForExternalContent to the same asset store and let us delete ~60 lines. That felt like scope creep for this PR — happy to file it.