ENG-1892 Add 'convert to' node option to right click menu for tldraw shape to allow user to convert shape to node (Obsidian) - #1432
Open
trangdoan982 wants to merge 2 commits into
Conversation
Extend the canvas right-click "Convert To" submenu beyond text and image shapes to geo (labelled rectangles, ellipses, etc.) and note (sticky) shapes, gated on the shape actually having text. Menu gating and converter dispatch now share canConvertShapeToNode so they cannot drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…adonly Rename the allowlist to RICH_TEXT_SHAPE_TYPES so it is obvious that getShapeText only reads props.richText -- adding a shape that stores its label elsewhere (arrow, frame) would otherwise silently do nothing. Also disable the Convert To items in readonly canvases, matching the Relation items, and note that images are gated at conversion time. Co-Authored-By: Claude Opus 5 <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. |
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
1 participant
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/012f47508e87410db4aaa383da36d25d
Reviewer brief
geoshapes (labelled rectangles, ellipses, etc.) andnote(sticky) shapes, not justtextandimage. It is gated on the shape actually having text, so an unlabelled rectangle shows nothing.arrowis deliberately excluded — it owns the separate "Relation" submenucreateDiscourseNodeShapepasses the shape's parent-relative x/y toeditor.createShapewithout aparentId, which treats them as page coordinates. Converting a shape parented to a frame would place the node at the wrong position. This is pre-existing and affects text and image conversions identically, so it is left out of this diff — but sticky notes and labelled rectangles inside frames are common, so this change makes it easier to hit. I did not independently reproduce it. Same for two other pre-existing items:markHistoryStoppingPointis called after the mutations rather than before, and an image whose asset does not resolve to a vault file converts to an empty-titled node.Verification
Driven against a running Obsidian over CDP, after confirming the loaded bundle was this branch's build:
Full conversion on a labelled rectangle: submenu listed every configured node type, the modal opened pre-filled with the shape's label, and confirming created the node file, removed the geo shape, and placed the node card at the original x/y.
pnpm install --frozen-lockfile+pnpm ci:validatepass (5/5 tasks, 0 cached).eslinton both changed files reports no errors and no warnings.Scope check
$scope-checkagainst the ENG ticket and final diff.Done When: readonly-mode gating and the allowlist rename came from the delegated review, not the ticket. Otherwise, the ticket'sDone Whenis empty and the parent (FEE-824) specifies only "convert tldraw shape to discourse node (if it has text in it)". The text/image case already shipped, so this change covers the remaining text-bearing shapes. No other behaviour changed; toast copy was generalised off "text" wording.Local delegated full review
useValuegate is genuinely reactive torichTextedits and cheap (rich-text rendering memoizes per object), that the allowlist matches the tldraw 3.14.2 schema exactly, and that the menu gate and converter dispatch cannot disagree. Its actionable points on this diff are applied in the second commit; the rest are the pre-existing items noted above.🤖 Generated with Claude Code