ENG-605: Add new relation flow

[trangdoan982]

Summary by CodeRabbit

• New Features

  • Added a Discourse Relation tool to create relation arrows between nodes with real-time drag updates and precise-drag after a short delay.
  • Highlights compatible targets and selects the created arrow automatically.

• UI/UX

  • Introduced a unified Discourse Tool Panel showing both node and relation types with dedicated buttons and icons.
  • Added cross-cursor feedback on activation and user-visible warnings for incompatible or invalid relations.
  • Improved selection handling so styles and panels reflect both node and relation tools.

[linear]

ENG-605 Add new relation flow

[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 ↗︎.

[trangdoan982]

• [ENG-603] Add existing relations #412
• ENG-605: Add new relation flow #411 👈 (View in Graphite)
• ENG-600: Discourse Relation shape definition #408
• tldraw-obsidian

This stack of pull requests is managed by Graphite. Learn more about stacking.

[trangdoan982]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (9)

apps/obsidian/src/components/canvas/DiscourseRelationTool.ts (4)

157-161: Remove unreachable branch after earlier null/Type guard

target is already asserted non-null and type-checked above; this if (!target) ... else block can’t hit the !target path. Simplify to just set hinting.

Apply this diff:

-    if (!target) {
-      this.createArrowShape();
-    } else {
-      this.editor.setHintingShapes([target.id]);
-    }
+    this.editor.setHintingShapes([target.id]);

---

80-111: Deduplicate and centralize relation compatibility logic

getCompatibleNodeTypes filters the same array twice and reimplements logic that likely belongs in a shared util. Consider moving this to a utility (e.g., relationUtils) and pre-indexing relations by type to avoid O(2n) scans on every pointer start.

I can extract this into a helper and add simple caching if helpful.

---

16-22: Global tool context can bleed across editors/views

relationToolContext is module-scoped. If multiple canvases/editors are open, context can be overwritten or cleared unexpectedly by another instance. Prefer storing context in the editor’s instance state or a store keyed by editor id.

I can wire this to editor.setInstanceState or a jotai/atom keyed by editor.instanceId if you want.

---

119-123: Avoid noisy notices when no relation type is set

Showing a Notice on any click with the tool active but no selected relation type can be distracting. Consider early-returning without a Notice and highlighting the panel to prompt selection.

apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (2)

205-212: Tool override OK; consider defaulting context on select

Overriding the "discourse-relation" tool is fine. Optional: if no relation type is focused, default-select the first available relation type to avoid “No relation type selected” warnings on first use.

---

233-252: Add a toolbar button for the relation tool

The panel covers selection, but parity in the toolbar improves discoverability and flow.

Apply this diff:

               Toolbar: (props) => {
                 const tools = useTools();
                 const isDiscourseNodeSelected = useIsToolSelected(
                   tools["discourse-node"],
                 );
+                const isDiscourseRelationSelected = useIsToolSelected(
+                  tools["discourse-relation"],
+                );
                 return (
                   <DefaultToolbar {...props}>
                     <TldrawUiMenuItem
                       id="discourse-node"
                       icon="discourseNodeIcon"
                       label="Discourse Node"
                       onSelect={() => {
                         if (editorRef.current) {
                           editorRef.current.setCurrentTool("discourse-node");
                         }
                       }}
                       isSelected={isDiscourseNodeSelected}
                     />
+                    <TldrawUiMenuItem
+                      id="discourse-relation"
+                      icon="tool-arrow"
+                      label="Discourse Relation"
+                      onSelect={() => {
+                        if (editorRef.current) {
+                          editorRef.current.setCurrentTool("discourse-relation");
+                        }
+                      }}
+                      isSelected={isDiscourseRelationSelected}
+                    />
                     <DefaultToolbarContent />
                   </DefaultToolbar>
                 );
               },

apps/obsidian/src/components/canvas/DiscourseToolPanel.tsx (3)

220-227: Cursor should also reflect relation focus

Cursor changes only when a node type is focused. Apply the same UX when a relation is focused.

Apply this diff:

-  useEffect(() => {
-    const cursor = focusedNodeTypeId ? "cross" : "default";
+  useEffect(() => {
+    const hasAnyFocus = !!focusedNodeTypeId || !!focusedRelationTypeId;
+    const cursor = hasAnyFocus ? "cross" : "default";
     editor.setCursor({ type: cursor });
     return () => {
       editor.setCursor({ type: "default" });
     };
-  }, [focusedNodeTypeId, editor]);
+  }, [focusedNodeTypeId, focusedRelationTypeId, editor]);

---

304-304: Remove redundant key on inner button

key should be on the mapped component, not the inner DOM element. It’s ignored here and can be dropped.

Apply this diff:

-    <button
-      key={nodeType.id}
+    <button

---

350-362: Avoid hard-coding CDN icon version

The mask URL pins 2.3.0; future TLDraw updates may break icons. Prefer a local asset or the editor’s icon system.

I can wire this to use assetUrls.icons or a local SVG import.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 65b4a9c and 6676abd.

📒 Files selected for processing (3)

• apps/obsidian/src/components/canvas/DiscourseRelationTool.ts (1 hunks)
• apps/obsidian/src/components/canvas/DiscourseToolPanel.tsx (8 hunks)
• apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check

🔇 Additional comments (4)

apps/obsidian/src/components/canvas/DiscourseRelationTool.ts (1)

182-187: Verify DiscourseRelationShape defines “end” handle
Search shows no getShapeHandles override or static handle list in DiscourseRelationShape.tsx. Ensure the shape (or its util) actually declares a handle with
• id: "end"
• type: vertex
• index: "a3"
before using it, or dragging behavior will be incorrect.

apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (2)

73-74: LGTM: tools registration

Registering both DiscourseNodeTool and DiscourseRelationTool in tools is correct.

---

217-231: LGTM: conditional StylePanel integration

Switching to DiscourseToolPanel when either discourse tool is selected works as intended.

apps/obsidian/src/components/canvas/DiscourseToolPanel.tsx (1)

242-251: Nice: relation tool handoff is wired

Clicking a relation type sets the context and switches the tool. Smooth flow.

[mdroidian]

Could you please add a loom video walking through the UI/UX changes then re-tag me? Thank you.

[trangdoan982]

oh yes my bad. here's the Loom link: https://www.loom.com/share/b1d6d2bc9261479287ed9a486326c93f

[mdroidian]

Found a couple bugs
https://www.loom.com/share/2d2056ae7c6e4a0db5b6d20d77119bf9

The one described in the video (relation tool being unselected) and also the relation arrow label line breaking. For the relation arrow line breaking, we had a similar issue in roam, you could look there in the css, might have the same solution.

[mdroidian]

@trangdoan982 another thing that we need to change is: relations need to be added on "mouseUp" not on mouseEnter. We noticed this in Roam canvas. This becomes an issue when you want to add a relation from nodeA to nodeB but nodeB is in a cluster of nodes ... it's like navigating a minefield on "mouseEnter".

[trangdoan982]

@mdroidian can you clarify what you mean by the mouse events? I think the flow for creating the relations in the app for Roam are quite different from Obsidian. For Roam, I noticed that the relation is created on "pointer_up" event and the function handleRelationCreation is added onMount. But for Obsidian, I only call checkAndReifyRelation() after the binding was created (onAfterCreate for the DiscourseRelationBinding.tsx, so my understanding is it doesn't rely on mouse events but tldraw events?!! But let me know if I'm missing something here

[mdroidian]

@trangdoan982 Yeah it was like this in Roam too for the initial upgrade , but changed. https://linear.app/discourse-graphs/issue/ENG-688/bind-on-mouse-up (I think it was changed before merging to main, so there's no PR to reference)

The binding gets created right when the handle is over a bindable shape. This causes a problematic UX. So we have to change when the relation is created to when the user releases the mouse. This way we know the intent of the user.

[trangdoan982]

@mdroidian yes I see. It created the wrong relation even. I pushed up the latest changes, which address this comment. Now the relation is only created on the pointer_up event, following similar pattern to Roam

[mdroidian]

A few bugs I noticed. None blocking, can be done later. Some not related to this PR.
https://www.loom.com/share/a839dd03460c4ca2866a03461d763d54

Let's make sure we host the .svg on vercel, nix the console.logs, and use named params on addRelationToFrontmatter, then feel free to merge