-
Notifications
You must be signed in to change notification settings - Fork 3
[ENG-624] TLDraw Obsidian asset store #326
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
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAsset store management was integrated into the TldrawView lifecycle, introducing creation, passing, and cleanup of an Obsidian-specific TLDraw asset store. The TldrawPreviewComponent now requires and utilizes this asset store. A new module implements the ObsidianTLAssetStore, handling asset upload, caching, and retrieval within the Obsidian vault and markdown files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TldrawView
participant ObsidianTLAssetStore
participant TldrawPreviewComponent
User->>TldrawView: Open file
TldrawView->>ObsidianTLAssetStore: Instantiate asset store with file context
TldrawView->>TldrawPreviewComponent: Create React root, pass assetStore
TldrawPreviewComponent->>ObsidianTLAssetStore: Use assetStore for assets
User->>TldrawView: Close or unload file
TldrawView->>ObsidianTLAssetStore: Dispose and clear asset store
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 3
🧹 Nitpick comments (5)
apps/obsidian/src/utils/asset-store.ts (2)
34-38: Avoid hard-coding the “attachments” folderThe path is baked in; users who changed their attachment location (or prefer per-vault settings) will get orphan files.
Pass the folder via plugin / workspace settings or fall back toapp.vault.getConfig("attachmentFolderPath")before creating one.
87-99:#addToTopOfFilerisks corrupting front-matter offsetsUsing
metadataCache.getFileCacheinside the sameprocesscall re-reads stale cache; front-matter positions may be wrong after the edit, producing duplicated front-matter or misplaced block ref lines.
Safer: parse the raw text once without relying on cache, or callmetadataCache.triggerCacheUpdate(file)after writing.apps/obsidian/src/components/TldrawView.tsx (3)
78-82:persistenceKeyshould be filesystem-safe
file.pathcontains slashes; some back-end stores treat/as a hierarchy delimiter.
Encode (e.g.encodeURIComponent(file.path)) or hash the path to avoid unexpected collisions.
121-122: Remove leftoverconsole.logDebug statement will spam the console for every canvas open.
168-170: Zero-delaysetTimeoutawaits is unnecessary
await new Promise(r => setTimeout(r, 0))adds one tick with no observable benefit here; React render is already synchronous forroot.render.
Drop the await unless you’re waiting for a subsequent measurement/layout step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/obsidian/src/components/TldrawView.tsx(7 hunks)apps/obsidian/src/components/TldrawViewComponent.tsx(3 hunks)apps/obsidian/src/utils/asset-store.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-07-19T22:34:16.794Z
Learning: Applies to apps/obsidian/Attachments/icons/*.svg : Store all icon files in the Attachments/icons folder
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to {apps/website,packages/ui}/**/*.{tsx,jsx} : When refactoring inline styles, use tailwind classes
Applied to files:
apps/obsidian/src/components/TldrawViewComponent.tsx
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to {apps/website,packages/ui}/**/*.{tsx,jsx} : Use platform-native UI components first with shadcn/ui as a fallback
Applied to files:
apps/obsidian/src/components/TldrawViewComponent.tsx
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to apps/website/**/*.{tsx,jsx,ts,js} : Use Tailwind CSS for styling where possible
Applied to files:
apps/obsidian/src/components/TldrawViewComponent.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/obsidian/src/components/TldrawViewComponent.tsx
📚 Learning: 2025-07-19T22:34:16.794Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-07-19T22:34:16.794Z
Learning: Applies to apps/obsidian/Attachments/icons/*.svg : Store all icon files in the Attachments/icons folder
Applied to files:
apps/obsidian/src/utils/asset-store.tsapps/obsidian/src/components/TldrawView.tsx
🔇 Additional comments (1)
apps/obsidian/src/components/TldrawView.tsx (1)
182-186: Double disposal is safe but redundant
assetStore.dispose()is invoked in bothonUnloadFileandonClose.
IfonClosealways followsonUnloadFile, one call suffices; otherwise leave as-is.
Consider guarding with a flag to avoid confusion.Also applies to: 214-218
mdroidian
left a 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.
Good stuff, a few changes requested.
Couple additional things to list here
-
I noticed the image gets added to the markdown with
!which "embeds" it. After a few images, this will get quite noisy. Let's remove the!. A user can preview with a modifier+hover if they want. -
I'd love to test this myself, but unfortunately I can't load it. I think this is related to the
cssissue we discussed.
|
@mdroidian i just pushed a fix for the styling issue, can you check if it works on your local now? |
No change for me. I even tried deleting We can debug it together tomorrow. |
mdroidian
left a 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.
🚀
* current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments
* current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments
* current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments
* current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments
* current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments
* [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/133f76d44b0b464192512721dd51f771?sid=bb166032-0937-4aa8-9c0c-d26f474bae5b
Summary by CodeRabbit
New Features
Bug Fixes