ENG-1356 Implement image to node conversion flow via icon button in tldraw Roam - #1427
Open
trangdoan982 wants to merge 5 commits into
Open
Conversation
…e nodes Hovering a tldraw image shape now shows a convert button that opens the create node dialog with the type picker enabled and an image preview. The existing right-click Convert To path is unchanged; its upload and shape-replacement steps move into shared helpers used by both flows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Skip locked image shapes, surface upload errors and unreplaced shapes as toasts, and tighten docs wording around Key Image. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Image insertion was gated on the Key Image canvas setting for new pages while the existing-node path added it unconditionally. Both conversion flows now share one helper that always writes the image block and uses Key Image only to pick the query-builder placeholder. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dropped and pasted canvas files are already uploaded to Roam and stored as the asset src, so conversion now reuses that URL and only uploads when the src is a data or blob URL. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Use the same Label as the other fields so the Image caption is bold, and render the preview at natural size flush left instead of centred. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mdroidian
requested changes
Sep 11, 2026
mdroidian
left a comment
Member
There was a problem hiding this comment.
To help the review, can you go through and write inline github comments on
- code sections (both removed and added) that are purely refactors with no functional change.
- code that is outside of the original scope
Could you also record a loom testing
- nodes that have Key Image defined via query block (making sure the image shows up in the correct location)
- the newly added image URL functionality
- testing the new changes to the right click flow
And finally, change the icon and icon positioning to match the design
- use
document-openicon - move to top right
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/ba1a4a782b3c4ac6b0e713bd4561a8fb
Note: I updated the styling to match Figma design after recording this Loom. "Image" is now highlighted and embeded image is flushed to the left:

Reviewer brief
ImageConvertOverlay.tsxhover tracking viaeditor.getHoveredShape()insideuseValue; the additivenodeTypefield in theModifyNodeDialog.tsxsuccess payload; andhandleImageCreationincreateDiscourseNode.ts, which now always writes the image block for a canvas conversion and uses Key Image only to choose the query-builder placeholder (previously the image was dropped when Key Image was off).72733d84a"Reuse the canvas asset URL instead of re-uploading the image". Files dropped or pasted onto the canvas are already uploaded to Roam by the canvas handlers and stored as the assetsrc, so conversion now reuses that URL and only re-uploads when thesrcis adata:orblob:URL. This affects both flows. If we prefer every conversion to create its own copy of the file, revert or drop that single commit; nothing else depends on it.deleteShapes. The canvas node shape still displays the image only for node types with Key Image on.Verification
pnpm install --frozen-lockfileandpnpm ci:validatepass (226 Roam unit tests).Scope check
$scope-checkagainst the ENG ticket and final diff.Done When: the always-add-image change and the URL reuse commit also apply to the existing right-click flow, because both flows share one implementation. Requested by Trang during review so both flows behave identically.Local delegated full review
$dg-delegated-full-reviewwhen no other full-review workflow is available.🤖 Generated with Claude Code