-
Notifications
You must be signed in to change notification settings - Fork 3
[ENG-928] Shift click to open file on the right split #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENG-928] Shift click to open file on the right split #485
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds SHIFT+click handling in TldrawViewComponent to open a linked Obsidian file from a Discourse Node. Implements debounce, detects single selected discourse-node under pointer, extracts block ref, resolves linked file, opens it in a split, clears selection, and shows toasts on missing refs/errors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as TldrawViewComponent
participant E as Tldraw Editor
participant X as BlockRef Extractor
participant R as Linked File Resolver
participant O as Obsidian App
participant T as Toast
U->>C: SHIFT+Click on canvas
C->>C: Debounce guard (skip if recent)
alt Debounced
C->>T: showToast("Please wait")
else Proceed
C->>E: get shape under pointer
E-->>C: discourse-node?
alt Not a discourse-node or multiple selection
C->>C: Exit (no action)
else Single discourse-node
C->>X: extract block ref
alt No block ref
C->>T: showToast("Missing block reference")
else Has block ref
C->>R: resolveLinkedTFileByBlockRef(ref)
alt Resolve error or not found
C->>T: showToast("Unable to resolve/open")
else Resolved TFile
C->>O: open file in right/vertical split
alt Open failed
C->>T: showToast("Open failed")
else Opened
C->>E: clear selection
end
end
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (1)
226-226: Remove redundant null coalescing.
shape.props.src ?? undefinedis redundant since the nullish coalescing operator already returnsundefinedwhen the left operand isnullorundefined.- const blockRefId = extractBlockRefId(shape.props.src ?? undefined); + const blockRefId = extractBlockRefId(shape.props.src);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/obsidian/src/components/canvas/TldrawViewComponent.tsx(4 hunks)
🔇 Additional comments (7)
apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (7)
33-41: LGTM! Clean imports for the new functionality.The new imports are well-organized and all are utilized in the shift-click handler implementation.
Also applies to: 53-53
71-72: LGTM! Good debounce implementation.The 300ms debounce window is reasonable for preventing accidental double-clicks while remaining responsive.
203-210: LGTM! Effective debounce logic.The timestamp-based debounce correctly prevents rapid successive shift-clicks within 300ms.
277-277: LGTM! Good UX touch.Clearing the editor selection after opening the file prevents confusion and keeps the UI clean.
228-286: LGTM! Comprehensive error handling with user feedback.The implementation provides clear user feedback through toast notifications for all error cases (missing block ref, file read error, file not found, failed open) and includes proper error logging.
217-224: Verify shift-click requires selected node
Current guard only checksselectedShapes.length, so if Node A is selected and you shift-click unselected Node B, it still opens B. Should the clicked node be required to be inselectedShapes?
246-286: Confirmworkspace.rightSplitis a valid API
I couldn’t findrightSplitin the Obsidian type definitions—verify it’s exposed in your target Obsidian version or replace withsplitActiveLeaf/alternative split APIs.
c649cf4 to
d16c9a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I misunderstood our last conversation where you mentioned this. I assumed the behavior was going to be opening the file in the workspace in another vertical tab beside the existing tab.
Seems like an odd behavior, to open a file in the sidebar. This goes against the plugins default behavior (shift click opens in current tab). This will also lead to some odd UX like if the canvas is in it's own window, for example. And closing a sidebar leaf isn't as clear as it is in the main window.
But, if you are sure this is what was requested, I won't block on it. I just couldn't find the original request in the ticket. I'd like to investigate the discussion and bring this up.
EDIT - I tested shift click on a property, shift click on a regular link does nothing
As long as we are matching Obsidian in the future:
- Ctrl - open in new tab in current leaf
- Alt+Ctrl - open in new leaf
and creating some affordance to open the node in the current tab, I'm happy.
|
I'll note this behavior down in this ticket: https://linear.app/discourse-graphs/issue/ENG-874/open-dn-file |
* [ENG-495] Tldraw obsidian setup (#285) * cleaned * sm * address PR comments * [ENG-598] Data persistence for tldraw (#303) * data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments * [ENG-624] TLDraw Obsidian asset store (#326) * current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments * correct styles * [ENG-599] Discourse node shape (#341) * current state * works now * clean up * address PR comments * address PR reviews * fix styling issues * latest progress * update new shape * shape defined * address PR comments * sm address PR review * current progress * reorg * address other PR comments * clean * simplify flow * address PR comments * [ENG-604] Create node flow (#387) * eng-604: create node flow * pwd * [ENG-658] Add existing node flow (#389) * eng-658-add-existing-nodes-flow * address PR comments * small changes * [ENG-601] Create settings for canvas and attachment default folder (#338) * add new settings * small add * ENG-600: Discourse Relation shape definition (#408) * ENG-605: Add new relation flow (#411) * [ENG-603] Add existing relations (#412) https://www.loom.com/share/3641f2a642714b0d849262344e8c6ee5?sid=0614c657-e541-4bfd-92df-9b1aa60945b6 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added a Relations overlay on the canvas that shows a “Relations” button when a discourse node is selected. - Introduced a Relations panel to view and manage relations for the selected node, including adding or removing links, with clear loading/error states. - Overlay appears above the canvas without disrupting existing tools. - Chores - Consolidated relation-type lookup into shared utilities and updated imports. No user-facing changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * [ENG-844] Add color setting for relation types (#429) * add color setting * address PR reviews * address PR commens * fix icons * ENG-812 Update of database cli tools (#401) * eng-812 : Update database cli tools: supabase, vercel, cucumber. * newer cucumber constrains node * [ENG-495] Tldraw obsidian setup (#285) * cleaned * sm * address PR comments * [ENG-598] Data persistence for tldraw (#303) * data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments * switch to pnpm * delete wrong rebase file * fix pnpm lock * fix type checks * address all the PR comments * delete redundant files * fix types * shift click to open file on the right split (#485) * address PR comments * final lint cleanup --------- Co-authored-by: Marc-Antoine Parent <maparent@acm.org>

https://www.loom.com/share/0865fab1de364937a0df36a0d07dbcd4
Summary by CodeRabbit