[Eng-928] Shift + Click to open discourse node

[trangdoan982]

Summary by CodeRabbit

• New Features
  • Shift-click on a discourse node now opens its linked note directly, with a brief debounce to prevent accidental repeats.
  • Clear in-app toasts provide feedback for success and error states (e.g., missing link, unavailable canvas cache, resolution failures).
  • Automatically clears the current selection after opening the linked note for a smoother workflow.

[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-928] Shift + Click to open discourse node #478 👈 (View in Graphite)
• tldraw-obsidian

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

[linear]

ENG-928 Shift + Click to open discourse node

[trangdoan982]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

Implements shift-click handling on canvas pointer_up to open a discourse-node’s linked file via block reference, with debounce, selection guards, cache checks, and toast-based feedback. Adds related imports, refs, and utilities. Converts TldrawPreviewProps from interface to type alias without changing fields.

Changes

Cohort / File(s)	Summary of changes
Shift-click discourse-node activation apps/obsidian/src/components/canvas/TldrawViewComponent.tsx	Added pointer_up shift-click flow: debounce via lastShiftClickRef/SHIFT_CLICK_DEBOUNCE_MS; detect DiscourseNodeShape under cursor; enforce single-node selection; extract blockRefId; read canvas file cache; resolve linked file via assetStore; open on success; clear selection; show toasts on missing/failed states; error logging.
Type and plumbing updates apps/obsidian/src/components/canvas/TldrawViewComponent.tsx	Converted interface TldrawPreviewProps → type alias TldrawPreviewProps with same fields (store, file, assetStore). Added imports: DiscourseNodeShape, extractBlockRefId, resolveLinkedTFileByBlockRef, showToast. Initialized saveTimeoutRef (nullable), editorRef (null default).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Canvas as TldrawViewComponent
  participant Store as TLStore
  participant Asset as ObsidianTLAssetStore
  participant Obsidian as App
  participant Toast as Toasts

  User->>Canvas: pointer_up (Shift held)
  alt Debounced within 300ms
    Canvas->>Toast: show "Please wait…" / ignore
  else Proceed
    Canvas->>Store: hit-test shape under cursor
    Store-->>Canvas: shape or none
    alt Not a DiscourseNode or none/multiple selected
      Canvas->>Toast: warn "Select a single discourse-node"
    else DiscourseNode selected
      Canvas->>Canvas: extractBlockRefId(node)
      alt No blockRefId
        Canvas->>Toast: warn "No block reference"
      else Has blockRefId
        Canvas->>Obsidian: read canvas file cache
        alt Cache unavailable
          Canvas->>Toast: error "Canvas cache unavailable"
        else Cache available
          Canvas->>Asset: resolveLinkedTFileByBlockRef(blockRefId, cache)
          alt Resolve success
            Asset-->>Canvas: TFile
            Canvas->>Obsidian: open file
            Obsidian-->>Canvas: opened
            Canvas->>Store: clear selection
          else Resolve failure
            Asset-->>Canvas: error/undefined
            Canvas->>Toast: error "Linked file not found"
          end
        end
      end
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [ENG-624] TLDraw Obsidian asset store #326: Earlier addition of the Obsidian TL asset store prop and wiring used here for block-ref resolution.
• ENG-861 - Update canvas node click behavior #449: Adjusts pointer_up shift-click behavior and selection guards, overlapping with the new activation flow.
• [ENG-599] Discourse node shape #341: Shares discourse-node utilities (extractBlockRefId, resolveLinkedTFileByBlockRef, DiscourseNodeShape) directly used in this change.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly identifies the main feature addition of enabling shift-click to open a discourse node and includes the ticket identifier, making it concise, specific, and directly related to the primary change.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

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: 1

🧹 Nitpick comments (1)

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

226-234: Redundant ?? undefined operation.

On line 226, shape.props.src ?? undefined is redundant. If shape.props.src is null or undefined, the nullish coalescing operator returns undefined, which is the same as just passing shape.props.src directly.

Apply this diff to simplify:

-          const blockRefId = extractBlockRefId(shape.props.src ?? undefined);
+          const blockRefId = extractBlockRefId(shape.props.src);

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47fd2e4 and cda1da2.

📒 Files selected for processing (1)

• apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (3 hunks)

🔇 Additional comments (7)

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

45-50: LGTM! Imports support the shift-click feature.

All new imports are used in the shift-click handler logic.

---

52-56: LGTM! Type alias conversion is semantically equivalent.

The conversion from interface to type alias has no functional impact. Both declarations define the same shape.

---

69-73: LGTM! Refs and debounce constant are well-defined.

The debounce interval of 300ms is appropriate for preventing accidental double-clicks. Explicit null initialization for saveTimeoutRef and editorRef provides clarity.

---

202-210: LGTM! Debouncing logic is correctly implemented.

The debounce mechanism properly prevents double-clicks within 300ms.

---

182-281: Well-integrated shift-click handler.

The shift-click functionality is cleanly integrated into the existing event handling system without disrupting the relation creation logic. The use of toasts for user feedback is consistent with the rest of the codebase.

---

246-277: Clarify newLeaf usage in openLinkText
You’re passing true to always open in a new pane; elsewhere (e.g. createNode.ts uses false, RelationshipSection.tsx omits the flag) calls default to the current pane. Confirm if forcing a new leaf here (TldrawViewComponent.tsx:262) is intentional or should match existing behavior.

---

217-224: Clarify shift-click open behavior
In the shift-click handler we bail when more than one discourse node is selected but don’t verify that the clicked node is the one selected. Should we only open when the clicked shape’s ID matches the single selected node (e.g. if (selectedDiscourseNodes[0].id !== shape.id) return;)?

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unsafe type assertion on shape.

The type assertion as DiscourseNodeShape on line 213 is unsafe. getShapeAtPoint returns a shape of unknown type, and while line 215 checks the type at runtime, TypeScript doesn't narrow the type from the assertion.

Consider narrowing the type after the runtime check:

-          const shape = editor.getShapeAtPoint(
-            editor.inputs.currentPagePoint,
-          ) as DiscourseNodeShape;
+          const shape = editor.getShapeAtPoint(
+            editor.inputs.currentPagePoint,
+          );

           if (!shape || shape.type !== "discourse-node") return;
+          
+          // Now we know shape is a discourse-node
+          const discourseNode = shape as DiscourseNodeShape;

Then use discourseNode instead of shape in the subsequent code.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const shape = editor.getShapeAtPoint(
	editor.inputs.currentPagePoint,
	) as DiscourseNodeShape;
	if (!shape || shape.type !== "discourse-node") return;
	const shape = editor.getShapeAtPoint(
	editor.inputs.currentPagePoint,
	);
	if (!shape || shape.type !== "discourse-node") return;
	// Now we know shape is a discourse-node
	const discourseNode = shape as DiscourseNodeShape;

🤖 Prompt for AI Agents

In apps/obsidian/src/components/canvas/TldrawViewComponent.tsx around lines
211-215, remove the unsafe inline cast "as DiscourseNodeShape" on the result of
getShapeAtPoint; instead keep the runtime type check (if (!shape || shape.type
!== "discourse-node") return;) and then narrow the type afterwards by creating a
new const like "const discourseNode = shape as DiscourseNodeShape" (or use a
type predicate helper) and use discourseNode in subsequent code—this preserves
the runtime guard and confines the assertion to a safe, post-check assignment.