ENG-1693 Add insert backlink checkbox to ModifyNodeModal#1018
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
8c687f1 to
076bf8a
Compare
9ef1a35 to
be20fdc
Compare
…efaults - Add "Insert backlink" checkbox to ModifyNodeModal (create mode only) - Default true when pre-filled text exists (user had text selected) or existing node is chosen; false otherwise - Gate backlink insertion in canvas flow and editor command flow on checkbox value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
be20fdc to
98af9b1
Compare
mdroidian
left a comment
There was a problem hiding this comment.
Requesting changes because this introduces a bug in the canvas create-node flow.
Broader design concern
Separate from the immediate bug, I’m worried about ModifyNodeModal continuing to accumulate workflow-specific props and submit fields.
The modal is now responsible for:
- create mode
- edit mode
- existing-node search
- relationship creation
- editor insertion
- canvas creation behavior
- backlink insertion
That makes the function hard to track because the meaning of a submit field depends heavily on which caller opened the modal.
It also makes bugs hard to debug. insertBacklink is a good example: in an editor flow, it means “insert a wikilink into the editor.” In the canvas flow, it ended up controlling whether the shape gets the blockref it needs to resolve to a file. Those are very different side effects hidden behind the same boolean.
I’d prefer we make caller capabilities explicit instead of inferring them inside the modal from global workspace state, and avoid using one generic submit payload for unrelated workflows. But we can continue that exploration in ENG-1750: Refactor Obsidian ModifyNodeModal contract for clearer workflow ownership
| canvasFile, | ||
| linkedFile: fileToUse, | ||
| }); | ||
| const src = insertBacklink |
There was a problem hiding this comment.
I believe this breaks canvas.
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ba4a87f7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
mdroidian
left a comment
There was a problem hiding this comment.
Could you submit a video showing that the bug as been fixed?
Also, please take a look at #1018 (comment)
Only editor flows that honor insertBacklink via createModifyNodeModalSubmitHandler show the toggle, fixing misleading UI in image conversion and tag-node flows. Co-authored-by: Cursor <cursoragent@cursor.com>
|
New test on editor and canvas: |
9847172
into
eng-1626-pre-fill-the-create-node-dialog-title-field-with-currently
https://www.loom.com/share/2c08071a85d849f0b13e828541a0a429
Summary
ModifyNodeModal(create mode only)truewhen the modal is opened with pre-filled text (user had text selected before invoking)truewhen the user selects an existing node from the searchfalsewhen opened with no selected text (blank create)nodeCreationFlow.ts) and the editor command flow (registerCommands.ts) on the checkbox valueTest plan
Closes ENG-1693
🤖 Generated with Claude Code