Tldraw obsidian #406
Tldraw obsidian #406
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
trangdoan982
force-pushed
the
tldraw-obsidian
branch
from
d3a64a3 to
e587380
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds a full Tldraw-based canvas feature to the Obsidian app: custom shapes (nodes/relations), tools, overlays, relation bindings, asset store, persistence and a new preview view. Introduces search and creation flows, frontmatter updates, settings and constants, utility modules, UI components, documentation, and minor updates across related areas. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TP as DiscourseToolPanel
participant DT as DiscourseNodeTool
participant NE as openCreateDiscourseNodeAt
participant AS as AssetStore
participant ED as Tldraw Editor
participant FS as File System
U->>TP: Select Node Type / drag on canvas
TP->>DT: setDiscourseNodeToolContext(plugin, file, nodeTypeId)
U->>ED: Pointer down (discourse-node tool)
ED->>DT: onPointerDown
DT->>NE: openCreateDiscourseNodeAt({ plugin, canvasFile, editor, position, initialNodeType })
NE->>FS: createDiscourseNode()
NE->>AS: addWikilinkBlockrefForFile(canvasFile, createdFile)
AS-->>NE: src (blockref)
NE->>ED: create shape(type: discourse-node, { src, title, nodeTypeId })
ED-->>U: Node appears and is selected
sequenceDiagram
autonumber
actor U as User
participant TP as DiscourseToolPanel
participant RT as DiscourseRelationTool
participant ED as Tldraw Editor
participant RB as RelationBindingUtil
participant FM as Frontmatter Utils
U->>TP: Select Relation Type
TP->>RT: setDiscourseRelationToolContext(plugin, relationTypeId, file)
U->>ED: Drag from node A to node B
ED->>RT: create arrow(shape: discourse-relation)
RT->>RB: bind start/end to nodes (validate compatibility)
ED-->>U: Arrow with label shows between nodes
U->>ED: Confirm/complete relation
ED->>RB: finalize bindings
ED->>FM: addRelationToFrontmatter(sourceFile, targetFile, relationTypeId)
FM-->>ED: Frontmatter updated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/obsidian/src/components/RelationshipSection.tsx (1)
197-215: Unify relationship writes via addRelationToFrontmatter; avoid duplicates and basename/extension mismatches.This block can create duplicate entries and ambiguous/incorrect links (e.g., [[Foo]] vs [[Foo.md]]). Reuse the new frontmatter util so linktext is canonical and de-duped.
Apply this diff:
- try { - const appendLinkToFrontmatter = async (file: TFile, link: string) => { - await plugin.app.fileManager.processFrontMatter(file, (fm) => { - const existingLinks = Array.isArray(fm[relationType.id]) - ? fm[relationType.id] - : []; - fm[relationType.id] = [...existingLinks, link]; - }); - }; - - await appendLinkToFrontmatter( - activeFile, - `"[[${selectedNode.name}]]"`.replace(/^['"]+|['"]+$/g, ""), - ); - await appendLinkToFrontmatter( - selectedNode, - `"[[${activeFile.name}]]"`.replace(/^['"]+|['"]+$/g, ""), - ); + try { + await addRelationToFrontmatter({ + app: plugin.app, + plugin, + sourceFile: activeFile, + targetFile: selectedNode, + relationTypeId: selectedRelationType, + });Add missing import at top of file:
+import { addRelationToFrontmatter } from "./canvas/utils/frontmatterUtils";
🧹 Nitpick comments (44)
apps/obsidian/tsconfig.json (1)
14-16: Use appropriate lib targets (include DOM.Iterable) or rely on base config."ES5","ES6","ES7" is unusual; React + tldraw typically need DOM.Iterable and a modern ES lib. Consider deferring to the base config or set a single modern target.
Apply:
- "lib": ["DOM", "ES5", "ES6", "ES7"] + "lib": ["DOM", "DOM.Iterable", "ES2020"]If the base config already sets libs correctly, remove this override instead.
apps/roam/src/components/DiscourseNodeSearchMenu.tsx (1)
197-214: Add error handling to silenced Promises.Using
voidsuppresses unhandled promise warnings but also swallows errors. Add.catchfor observability.Apply:
- void waitForBlock(blockUid, textarea.value).then(() => { + void waitForBlock(blockUid, textarea.value).then(() => { onClose(); @@ - void updateBlock({ uid: blockUid, text: newText }).then(() => { + void updateBlock({ uid: blockUid, text: newText }) + .then(() => { const newCursorPosition = triggerPosition + pageRef.length; @@ - if (window.roamAlphaAPI.ui.setBlockFocusAndSelection) { - void window.roamAlphaAPI.ui.setBlockFocusAndSelection({ + if (window.roamAlphaAPI.ui.setBlockFocusAndSelection) { + void window.roamAlphaAPI.ui.setBlockFocusAndSelection({ location: { "block-uid": blockUid, "window-id": windowId, }, selection: { start: newCursorPosition }, - }); + }).catch(console.error); } else { setTimeout(() => { const textareaElements = document.querySelectorAll("textarea"); for (const el of textareaElements) { if ( getUids(el as HTMLTextAreaElement).blockUid === blockUid ) { (el as HTMLTextAreaElement).focus(); (el as HTMLTextAreaElement).setSelectionRange( newCursorPosition, newCursorPosition, ); break; } } }, 50); } - }); + }) + .catch(console.error); posthog.capture("Discourse Node: Selected from Search Menu", { id: item.id, text: item.text, }); }, 10); - }); + }).catch(console.error);apps/obsidian/src/utils/file.ts (2)
3-12: Make folder creation resilience-safe under races.Between check and create, another process can create the folder. Swallow EEXIST by rechecking after failure.
Apply:
export const checkAndCreateFolder = async (folderpath: string, vault: Vault) => { if (!folderpath) return; const abstractItem = vault.getAbstractFileByPath(folderpath); if (abstractItem instanceof TFolder) return; if (abstractItem instanceof TAbstractFile) { throw new Error(`${folderpath} exists as a file`); } - await vault.createFolder(folderpath); + try { + await vault.createFolder(folderpath); + } catch (e) { + // If it now exists as a folder, ignore race; otherwise rethrow. + const now = vault.getAbstractFileByPath(folderpath); + if (!(now instanceof TFolder)) throw e; + } };
23-24: Handle empty folderpath when constructing fname.Avoid leading slashes at repo root by omitting the folder segment when empty.
Apply:
- let fname = normalizePath(`${folderpath}/${filename}`); + let fname = normalizePath(folderpath ? `${folderpath}/${filename}` : filename);apps/obsidian/src/components/canvas/DiscourseRelationTool.ts (2)
167-172: Unreachable branch after earlier validation.After returning when
!target || target.type !== "discourse-node",targetis always truthy here; theif (!target)block never runs.Apply:
- if (!target) { - this.createArrowShape(); - } else { - this.editor.setHintingShapes([target.id]); - } + this.editor.setHintingShapes([target.id]);
19-27: Global mutable tool context can clash across canvases.Consider scoping context per editor/canvas instance to avoid cross‑file interference.
apps/obsidian/package.json (1)
33-38: Pin tldraw to exact version
Replace in package.json:- "tldraw": "^3.14.2" + "tldraw": "3.14.2"React 19 support was added in v3.8.0 with no known incompatibilities in 3.14.x; you may still see peer-dependency warnings from ecosystem libs (e.g. Radix). Optionally pin date-fns to
"4.1.0"for reproducibility.apps/obsidian/src/components/canvas/utils/toastUtils.ts (1)
13-20: Avoid potential toast ID collisions.
tldrawtoasts dedupe by id; two same-severity toasts fired in the same millisecond will share the current id and the later one wins. Please salt the id with a random suffix (or usecrypto.randomUUID) to keep rapid-fire toasts distinct.- id: `${severity}-${Date.now()}`, + id: `${severity}-${Date.now()}-${Math.random().toString(36).slice(2)}`,apps/obsidian/src/components/canvas/utils/frontmatterUtils.ts (2)
32-42: Type the frontmatter arg correctly for processFrontMatter.FrontMatterCache is a read-cache type; processFrontMatter receives a mutable record. The diff above switches to Record<string, unknown>. Also remove the now-unused FrontMatterCache import.
Additional change outside this hunk:
-import type { App, FrontMatterCache, TFile } from "obsidian"; +import type { App, TFile } from "obsidian";
25-28: Prefer user-facing feedback over silent console error.Returning early on missing relation type is fine, but consider a Notice/toast in UI contexts.
apps/obsidian/src/components/canvas/utils/nodeCreationFlow.ts (1)
43-56: Center the created shape at the target position.Current x/y place the shape’s top-left at the position. Centering improves UX and matches typical TLDraw behavior.
Apply this diff:
- const shapeId = createShapeId(); + const shapeId = createShapeId(); + const width = 200; + const height = 100; tldrawEditor.createShape({ id: shapeId, type: "discourse-node", - x: position.x, - y: position.y, + x: position.x - width / 2, + y: position.y - height / 2, props: { - w: 200, - h: 100, + w: width, + h: height, src: src ?? "", title: createdFile.basename, nodeTypeId: selectedNodeType.id, }, });apps/obsidian/src/components/RelationshipSection.tsx (3)
96-99: Fallback when complement is missing.relationType.complement may be empty. Prefer complement ?? label.
Apply this diff:
- label: isSource ? relationType.label : relationType.complement, + label: isSource ? relationType.label : (relationType.complement ?? relationType.label),
192-233: Consider notifying the user on missing relation type.Short-circuiting is fine; a Notice would improve UX.
1-12: Standardize linktext across frontmatter assignments
- apps/obsidian/src/utils/templates.ts (line 138)
- apps/obsidian/src/utils/createNode.ts (lines 64, 167)
- apps/obsidian/src/components/RelationshipSection.tsx (line 199)
- apps/obsidian/src/components/canvas/utils/frontmatterUtils.ts (line 32)
- apps/obsidian/src/components/BulkIdentifyDiscourseNodesModal.tsx (line 128)
Replace these direct fm[relationType.id] writes with the canonical linktext util and run a one-time migration to consolidate legacy '[[name]]' entries into the new '[[linktext]]' format.
apps/obsidian/src/components/SearchBar.tsx (2)
63-68: Clear pending debounce on close to avoid stray timers.Override close() to clear debounce timeout.
Apply this diff:
selectSuggestion(item: T, evt: MouseEvent | KeyboardEvent): void { this.textInputEl.value = this.getItemTextFn(item); this.onSelectCallback(item); this.close(); } + + override close(): void { + if (this.debounceTimeout) { + clearTimeout(this.debounceTimeout); + this.debounceTimeout = null; + } + super.close(); + } }
97-121: Minor: effect can avoid re-instantiating GenericSuggest on asyncSearch changes.You already keep asyncSearchRef current. Removing asyncSearch from deps avoids churn.
apps/obsidian/src/components/canvas/DiscourseToolPanel.tsx (4)
132-142: Normalize data- attribute naming for dataset access.*Use kebab-case data attribute and camelCase dataset access for clarity and cross-browser consistency.
- const nodeTypeId = target.dataset.dg_node_type_id!; + const nodeTypeId = target.dataset.dgNodeTypeId!;- data-dg_node_type_id={nodeType.id} + data-dg-node-type-id={nodeType.id}Also applies to: 319-325
198-203: Parity check for focusedRelationTypeId.Mirror the node-type existence guard for relation types to avoid stale focus if a relation type is removed.
useEffect(() => { if (!focusedNodeTypeId) return; const exists = !!getNodeTypeById(plugin, focusedNodeTypeId); if (!exists) setFocusedNodeTypeId(undefined); }, [focusedNodeTypeId, plugin]); + + useEffect(() => { + if (!focusedRelationTypeId) return; + const exists = !!relationTypes.find((r) => r.id === focusedRelationTypeId); + if (!exists) setFocusedRelationTypeId(undefined); + }, [focusedRelationTypeId, relationTypes]);
50-146: Clean up listeners on unmount.If the component unmounts mid-drag, the document keydown listener can leak. Return a cleanup that calls your removeEventListeners (expose it via handlers or a ref).
327-333: Optional: add WebKit mask for Safari.For the masked color icon, add WebkitMask for Safari compatibility.
style={{ - mask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/color.svg") center 100% / 100% no-repeat`, + mask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/color.svg") center 100% / 100% no-repeat`, + WebkitMask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/color.svg") center 100% / 100% no-repeat`, backgroundColor: nodeType.color || "black", }}apps/obsidian/src/components/canvas/shapes/discourseNodeShapeUtils.ts (1)
20-25: Type relations once the schema is known.Replace unknown[] with a typed structure when you define the schema to avoid unsafe casts downstream.
apps/obsidian/src/components/canvas/TldrawView.tsx (4)
1-5: Import constants for Markdown view and TL data delimiters.Preps for the changes below.
-import { VIEW_TYPE_TLDRAW_DG_PREVIEW } from "~/constants"; +import { + VIEW_TYPE_TLDRAW_DG_PREVIEW, + VIEW_TYPE_MARKDOWN, + TLDATA_DELIMITER_START, + TLDATA_DELIMITER_END, +} from "~/constants";
52-57: Use the exported Markdown view constant.Removes a magic string.
- this.leaf.setViewState({ - type: "markdown", - state: this.leaf.view.getState(), - }), + this.leaf.setViewState({ + type: VIEW_TYPE_MARKDOWN, + state: this.leaf.view.getState(), + }),
70-71: Avoid double-reading the file.Use this.data populated by super.onLoadFile(file).
- const fileData = await this.app.vault.read(file); + const fileData = this.data || (await this.app.vault.read(file));
145-148: Remove unreachable check.The second assetStore check can’t run after the earlier throws.
- if (!this.assetStore) { - console.warn("Asset store is not set"); - return; - }apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx (1)
11-16: Also import getNodeTypeIdFromFrontmatter.Needed to derive nodeTypeId from frontmatter.
import { getFrontmatterForFile, FrontmatterRecord, + getNodeTypeIdFromFrontmatter, } from "./discourseNodeShapeUtils";apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (2)
174-195: Unsubscribe editor event listener to prevent leaks on unmount.Store the unsubscribe and clean it up when the component unmounts.
Apply these changes:
const handleMount = (editor: Editor) => { editorRef.current = editor; - - editor.on("event", (event) => { + const off = editor.on("event", (event) => { const e = event as TLPointerEventInfo; if (e.type === "pointer" && e.name === "pointer_down") { - const currentTool = editor.getCurrentTool(); - const currentToolId = currentTool.id; + const currentToolId = + // prefer the API that returns tool id; fallback if not available + // @ts-expect-error narrow at runtime + typeof (editor as any).getCurrentToolId === "function" + ? (editor as any).getCurrentToolId() + : (editor as any).getCurrentTool?.()?.id; if (currentToolId === "discourse-relation") { isCreatingRelationRef.current = true; } } if (e.type === "pointer" && e.name === "pointer_up") { if (isCreatingRelationRef.current) { BaseRelationBindingUtil.checkAndReifyRelation(editor); isCreatingRelationRef.current = false; } } }); + // Ensure we clean up when Tldraw unmounts this editor instance + editor.on("dispose", () => { + off?.(); + }); };If Editor does not emit "dispose", capture
offin a ref and clean it up via a React effect on unmount.
180-183: Use editor.getCurrentToolId() directly
Replace the two‐step call togetCurrentTool()with a single call:- const currentTool = editor.getCurrentTool(); - const currentToolId = currentTool.id; + const currentToolId = editor.getCurrentToolId();at apps/obsidian/src/components/canvas/TldrawViewComponent.tsx:180-183.
apps/obsidian/src/services/QueryEngine.ts (2)
61-66: Normalize DC API call to support both sync and async implementations.Some Datacore builds return arrays, others may return Promises. Normalize with Promise.resolve.
- const potentialNodes = await this.dc.query(dcQuery); + const potentialNodes = await Promise.resolve(this.dc.query(dcQuery));
110-112: Same normalization for DC query here.- const potentialNodes = this.dc.query(dcQuery); + const potentialNodes = await Promise.resolve(this.dc.query(dcQuery));apps/obsidian/src/components/canvas/utils/tldraw.ts (1)
195-205: Escape delimiters when building the regex.Even though these are constants, escaping prevents surprises and addresses ReDoS lint warning.
export const getUpdatedMdContent = ( currentContent: string, stringifiedData: string, ) => { - const regex = new RegExp( - `${TLDATA_DELIMITER_START}([\\s\\S]*?)${TLDATA_DELIMITER_END}`, - ); + const escape = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const regex = new RegExp( + `${escape(TLDATA_DELIMITER_START)}([\\s\\S]*?)${escape(TLDATA_DELIMITER_END)}` + ); return currentContent.replace( regex, `${TLDATA_DELIMITER_START}\n${stringifiedData}\n${TLDATA_DELIMITER_END}`, ); };apps/obsidian/src/components/canvas/stores/assetStore.ts (1)
41-53: Frontmatter insertion can duplicate leading newlines.Current splice always inserts
\n${content}\neven when no frontmatter exists (offsets 0). Consider normalizing to avoid unnecessary blank lines.apps/obsidian/src/components/canvas/shapes/DiscourseRelationShape.tsx (1)
65-77: Avoid duplicate handle enums.You import
ARROW_HANDLESand also defineArrowHandles. Prefer a single source to reduce drift.apps/obsidian/src/components/canvas/utils/relationUtils.tsx (2)
1385-1390: Reject the promise when 2D context is unavailable.Currently it neither resolves nor rejects, leaving a hanging promise.
Apply this diff:
- const ctx = canvasEl.getContext("2d"); - if (!ctx) return; + const ctx = canvasEl.getContext("2d"); + if (!ctx) { + reject(new Error("2D canvas context not available")); + return; + }
854-862: Remove duplicated guard.The second identical check is dead code and adds noise.
Apply this diff:
- if (!intersectionPoints || intersectionPoints.length !== 2) { - return { start: 0.5, end: 0.5 }; - } - - // there should be two intersection points - one near the start, and one near the end - // after the existing guard: - if (!intersectionPoints || intersectionPoints.length !== 2) - return { start: 0.5, end: 0.5 }; + if (!intersectionPoints || intersectionPoints.length !== 2) { + return { start: 0.5, end: 0.5 }; + }apps/obsidian/src/utils/registerCommands.ts (2)
83-99: Avoid passing stale view state across view typesUsing leaf.view.getState() when switching to markdown may carry incompatible state. Prefer setting only the required state (file) and mode.
Apply this diff:
- if (!checking) { - leaf.setViewState({ - type: VIEW_TYPE_MARKDOWN, - state: leaf.view.getState(), - }); - } + if (!checking) { + const file = (leaf as any)?.view?.file?.path ?? plugin.app.workspace.getActiveFile()?.path; + leaf.setViewState({ + type: VIEW_TYPE_MARKDOWN, + state: file ? { file } : {}, + }); + }
100-115: Ensure correct state shape when switching to Canvas viewSame concern as above; avoid reusing the prior view's state blob.
Apply this diff:
- if (!checking) { - leaf.setViewState({ - type: VIEW_TYPE_TLDRAW_DG_PREVIEW, - state: leaf.view.getState(), - }); - } + if (!checking) { + const file = (leaf as any)?.view?.file?.path ?? plugin.app.workspace.getActiveFile()?.path; + leaf.setViewState({ + type: VIEW_TYPE_TLDRAW_DG_PREVIEW, + state: file ? { file } : {}, + }); + }apps/obsidian/src/components/canvas/ToastListener.tsx (1)
4-8: Namespace the custom event to avoid collisionsUse a namespaced event name to reduce risk of clashing with other listeners.
- document.dispatchEvent( - new CustomEvent<TLUiToast>("show-toast", { detail: toast }), - ); + document.dispatchEvent( + new CustomEvent<TLUiToast>("dg:show-toast", { detail: toast }), + );Remember to update the listener below accordingly.
apps/obsidian/src/utils/utils.ts (1)
11-18: Defensive access and potential cachingMirror the defensive pattern above. If this runs on hot paths, consider Map caching keyed by id to avoid repeated linear scans.
- return plugin.settings.relationTypes.find( - (relation) => relation.id === relationTypeId, - ); + return (plugin.settings.relationTypes ?? []).find( + (relation) => relation.id === relationTypeId, + );apps/obsidian/src/components/canvas/overlays/RelationOverlay.tsx (2)
66-79: Improve a11y and stacking of the floating buttonAdd aria-label and ensure it stacks above TLDraw UI.
- <button + <button + aria-label="Open relations panel" onClick={handleOpen} style={{ left: `${buttonPosition.left}px`, top: `${buttonPosition.top}px`, transform: "translate(-50%, -100%)", pointerEvents: "all", + zIndex: 100, }} - className="absolute z-10 rounded px-3 py-1 text-xs text-white" + className="absolute rounded px-3 py-1 text-xs text-white" >
81-103: Panel position UXPanel is fixed at top-left (12,12). Consider anchoring near the selected node (like the button) for spatial continuity, or provide a setting.
apps/obsidian/src/components/canvas/DiscourseNodeTool.ts (1)
28-51: Await creation flow and clear context on exit/cancel pathsIf openCreateDiscourseNodeAt is async, not awaiting may cause race conditions when switching tools immediately after. Also ensure context is cleared if creation throws.
- override onPointerDown = (_info?: TLPointerEventInfo) => { + override onPointerDown = async (_info?: TLPointerEventInfo) => { @@ - openCreateDiscourseNodeAt({ + try { + await openCreateDiscourseNodeAt({ plugin, canvasFile, tldrawEditor: this.editor, position: currentPagePoint, initialNodeType, - }); - - toolContext = null; - this.editor.setCurrentTool("select"); + }); + } finally { + toolContext = null; + this.editor.setCurrentTool("select"); + }Also add:
override onExit = () => { toolContext = null; };apps/obsidian/src/components/RelationshipTypeSettings.tsx (2)
15-29: Initialize missing color field for existing settingsExisting relationTypes may lack color, leading to undefined value in the color input. Initialize defaults on first access.
You can normalize when mutating a missing index (as done), and also normalize initial state:
// when initializing state (outside selected lines) // const [relationTypes, setRelationTypes] = useState(...).map(rt => ({ color: "#000000", ...rt }));
135-143: Prevent uncontrolled color input when color is missingFallback to a default color to avoid React warnings and ensure a valid value.
- <input + <input type="color" - value={relationType.color} + value={relationType.color ?? "#000000"} onChange={(e) => handleRelationTypeChange(index, "color", e.target.value) } className="w-12 h-8 rounded border" title="Relation color" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
apps/website/public/apps/assets/node-color-icon.svgis excluded by!**/*.svgapps/website/public/apps/assets/tool-arrow-icon.svgis excluded by!**/*.svgapps/website/public/docs/obsidian/adding-existing-relations.gifis excluded by!**/*.gifapps/website/public/docs/obsidian/canvas-icon-button.pngis excluded by!**/*.pngapps/website/public/docs/obsidian/create-canvas-command.pngis excluded by!**/*.pngapps/website/public/docs/obsidian/create-discourse-node.gifis excluded by!**/*.gifapps/website/public/docs/obsidian/create-relations.gifis excluded by!**/*.gifapps/website/public/docs/obsidian/export-options.pngis excluded by!**/*.pngapps/website/public/docs/obsidian/node-search.gifis excluded by!**/*.gifapps/website/public/docs/obsidian/open-canvas-command.pngis excluded by!**/*.pngapps/website/public/docs/obsidian/relation-error.pngis excluded by!**/*.pngapps/website/public/docs/obsidian/search-filtering.gifis excluded by!**/*.gifpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
apps/obsidian/package.json(1 hunks)apps/obsidian/src/components/BulkIdentifyDiscourseNodesModal.tsx(2 hunks)apps/obsidian/src/components/DiscourseContextView.tsx(2 hunks)apps/obsidian/src/components/GeneralSettings.tsx(3 hunks)apps/obsidian/src/components/RelationshipSection.tsx(3 hunks)apps/obsidian/src/components/RelationshipSettings.tsx(4 hunks)apps/obsidian/src/components/RelationshipTypeSettings.tsx(5 hunks)apps/obsidian/src/components/SearchBar.tsx(2 hunks)apps/obsidian/src/components/canvas/DiscourseNodeTool.ts(1 hunks)apps/obsidian/src/components/canvas/DiscourseRelationTool.ts(1 hunks)apps/obsidian/src/components/canvas/DiscourseToolPanel.tsx(1 hunks)apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx(1 hunks)apps/obsidian/src/components/canvas/TldrawView.tsx(1 hunks)apps/obsidian/src/components/canvas/TldrawViewComponent.tsx(1 hunks)apps/obsidian/src/components/canvas/ToastListener.tsx(1 hunks)apps/obsidian/src/components/canvas/overlays/RelationOverlay.tsx(1 hunks)apps/obsidian/src/components/canvas/overlays/RelationPanel.tsx(1 hunks)apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx(1 hunks)apps/obsidian/src/components/canvas/shapes/DiscourseRelationBinding.tsx(1 hunks)apps/obsidian/src/components/canvas/shapes/DiscourseRelationShape.tsx(1 hunks)apps/obsidian/src/components/canvas/shapes/discourseNodeShapeUtils.ts(1 hunks)apps/obsidian/src/components/canvas/stores/assetStore.ts(1 hunks)apps/obsidian/src/components/canvas/utils/frontmatterUtils.ts(1 hunks)apps/obsidian/src/components/canvas/utils/nodeCreationFlow.ts(1 hunks)apps/obsidian/src/components/canvas/utils/relationUtils.tsx(1 hunks)apps/obsidian/src/components/canvas/utils/tldraw.ts(1 hunks)apps/obsidian/src/components/canvas/utils/toastUtils.ts(1 hunks)apps/obsidian/src/constants.ts(2 hunks)apps/obsidian/src/index.ts(3 hunks)apps/obsidian/src/services/QueryEngine.ts(4 hunks)apps/obsidian/src/types.ts(2 hunks)apps/obsidian/src/utils/createNode.ts(2 hunks)apps/obsidian/src/utils/file.ts(1 hunks)apps/obsidian/src/utils/registerCommands.ts(2 hunks)apps/obsidian/src/utils/utils.ts(1 hunks)apps/obsidian/styles.css(1 hunks)apps/obsidian/tsconfig.json(1 hunks)apps/roam/src/components/DiscourseNodeSearchMenu.tsx(2 hunks)apps/roam/src/components/canvas/DiscourseToolPanel.tsx(4 hunks)apps/roam/src/components/settings/Settings.tsx(1 hunks)apps/website/app/(docs)/docs/obsidian/navigation.ts(1 hunks)apps/website/app/(docs)/docs/obsidian/pages/canvas.md(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-06-17T23:37:45.289Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `type` serves as the unique identifier field, not a type classification field. The interface has no `uid` or `id` field, making `node.type` the correct field to use for UID-related operations.
Applied to files:
apps/obsidian/src/utils/utils.tsapps/obsidian/src/components/DiscourseContextView.tsxapps/obsidian/src/components/RelationshipSection.tsx
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Applied to files:
apps/roam/src/components/DiscourseNodeSearchMenu.tsxapps/obsidian/src/services/QueryEngine.ts
📚 Learning: 2025-08-25T15:53:21.799Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#372
File: apps/roam/src/components/DiscourseNodeMenu.tsx:116-116
Timestamp: 2025-08-25T15:53:21.799Z
Learning: In apps/roam/src/components/DiscourseNodeMenu.tsx, when handling tag insertion, multiple leading hashtags (like ##foo) should be preserved as they represent user intent, not normalized to a single hashtag. The current regex /^#/ is correct as it only removes one leading # before adding one back, maintaining any additional hashtags the user intended.
Applied to files:
apps/roam/src/components/DiscourseNodeSearchMenu.tsx
📚 Learning: 2025-06-17T23:37:45.289Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `node.type` serves as the UID field rather than having a conventional `node.uid` field. This is an unusual naming convention where the type field actually contains the unique identifier.
Applied to files:
apps/obsidian/src/components/DiscourseContextView.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/package.json : Prefer existing dependencies from package.json when adding or using dependencies in the Obsidian plugin
Applied to files:
apps/obsidian/package.json
📚 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/package.json
📚 Learning: 2025-08-27T02:23:45.696Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#377
File: packages/ui/tsconfig.json:5-7
Timestamp: 2025-08-27T02:23:45.696Z
Learning: When reviewing TypeScript configuration changes, check if settings like "declaration" are inherited from base configs before suggesting to add them explicitly. Many monorepo packages extend base configurations that already include necessary compiler options.
Applied to files:
apps/obsidian/tsconfig.json
📚 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 **/*.{ts,tsx,js,jsx} : Use Prettier with the project's configuration
Applied to files:
apps/obsidian/tsconfig.json
🪛 Biome (2.1.2)
apps/obsidian/src/components/canvas/utils/relationUtils.tsx
[error] 1788-1788: This code will never be reached ...
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 558-558: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ast-grep (0.39.5)
apps/obsidian/src/components/canvas/utils/tldraw.ts
[warning] 198-200: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
${TLDATA_DELIMITER_START}([\\s\\S]*?)${TLDATA_DELIMITER_END},
)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (30)
apps/obsidian/src/utils/createNode.ts (1)
1-1: Clean import updateNice catch removing the unused
Appimport; keeps the module lean.apps/roam/src/components/settings/Settings.tsx (1)
181-182: Verify CSS class existence for new Tab styling.Ensure
rm-settings__tabis defined and loaded in this context; otherwise, the Tab may lose expected styling.apps/website/app/(docs)/docs/obsidian/navigation.ts (1)
58-61: Canvas docs link fits the Core Features cluster.Thanks for surfacing the new Canvas page in the docs nav—it keeps the Core Features section aligned with the product surface.
apps/obsidian/src/components/BulkIdentifyDiscourseNodesModal.tsx (1)
220-245: Appreciate the centralized node type lookup.Routing everything through
getNodeTypeByIdkeeps the modal consistent with the rest of the refactor.apps/obsidian/src/components/DiscourseContextView.tsx (1)
43-74: Consistent node-type resolution is a nice improvement.This keeps the context view in step with the shared lookup helper and avoids hand-rolled searches.
apps/obsidian/src/types.ts (1)
17-34: Confirm migration path for the new relation-type color field.Existing user vaults will have relation types serialized without a
colorproperty. Please double-check thatloadSettings(or an explicit migration) backfills a default color before anything expects the new field; otherwise freshly upgraded users may hitundefinedcolors in the canvas or settings views.apps/obsidian/src/components/RelationshipSettings.tsx (1)
68-223: Looks good incorporating the shared helper.The settings UI now benefits from the same lookup logic as the rest of the canvas work.
apps/roam/src/components/canvas/DiscourseToolPanel.tsx (1)
47-373: Central icon constants keep things tidy.Nice touch extracting those masks—easier to maintain if the asset URL ever changes.
apps/obsidian/src/components/GeneralSettings.tsx (1)
72-183: General settings additions look solid.The folder pickers and state wiring blend right in with the existing pattern.
apps/obsidian/src/components/canvas/utils/nodeCreationFlow.ts (1)
18-74: Flow looks solid.Creation, wikilink insertion, shape add, history, and selection are wired correctly with defensive error handling.
apps/obsidian/src/components/RelationshipSection.tsx (3)
63-66: Node-type resolution via getNodeTypeById looks good.Using a single helper makes this consistent and safer.
368-423: Parsing existing relations is consistent with the new storage format.Regex extraction and link resolution via getFirstLinkpathDest are fine.
122-173: Good guardrails and updated QueryEngine call signature.Error states, guards, and the new options-based searchCompatibleNodeByTitle usage look correct.
apps/obsidian/src/components/SearchBar.tsx (1)
130-151: UX tweaks LGTM.Input styling, blur on select, and className passthrough look good.
apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx (1)
23-28: Search wiring LGTM.Engine instantiation and typed callbacks look fine.
apps/obsidian/src/constants.ts (2)
25-43: LGTM: relation colors.Adding color to relation types improves UI clarity and is backward compatible.
70-85: LGTM: new constants and defaults.Frontmatter key, TL data delimiters, view types, save delay, and asset URLs look coherent with the new flow.
apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx (1)
49-57: LGTM: default props and schema.Defaults and prop schema are sensible for a box-based shape util.
apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (1)
152-172: Auto-save debounce looks good.Debounced, user-scoped document changes; timeout cleared on cleanup. Good.
apps/obsidian/src/components/canvas/shapes/DiscourseRelationBinding.tsx (1)
140-171: Reification guard is solid.The reifiedArrows set prevents duplicate writes; errors remove from the set for retry. Looks good.
apps/obsidian/src/services/QueryEngine.ts (1)
246-253: Guard against missing $path.Nice addition; prevents getAbstractFileByPath(undefined) calls during bulk scan. LGTM.
apps/obsidian/src/components/canvas/overlays/RelationPanel.tsx (2)
315-335: Deleting the relation shape may leave frontmatter stale.Confirm whether deletion should also update frontmatter. If so, mirror creation flow by removing the relation from frontmatter.
Would you like me to wire this up using your frontmatter utilities (e.g., a removeRelation helper) so canvas and metadata stay consistent?
483-543: Grouping logic and link resolution look good.Correctly maps frontmatter relation arrays to files, dedupes, and respects directionality. LGTM.
apps/obsidian/src/components/canvas/utils/tldraw.ts (1)
44-87: Store initialization looks correct.Custom shape and binding utils registered; initialData normalization handles array/object records. Good.
apps/obsidian/src/components/canvas/stores/assetStore.ts (1)
260-279: Object URL caching looks solid.Nice use of
URL.revokeObjectURLon replacement and indispose()to avoid leaks.apps/obsidian/src/components/canvas/utils/relationUtils.tsx (1)
1703-1716: Good: translation start state captured in a WeakMap.Using
WeakMapfor ephemeral shape state avoids leaks while enabling smooth unbinding/rebinding during translate.apps/obsidian/src/components/RelationshipTypeSettings.tsx (1)
85-91: Validation OKRequiring color alongside id/label/complement is consistent with the new field.
apps/website/app/(docs)/docs/obsidian/pages/canvas.md (1)
105-107: Confirm frontmatter flag key
No matches found fortldr-dgor similar in the codebase—please verify the exact frontmatter key your canvas view plugin expects (e.g.tldraw-dgor another prefix) and update the docs accordingly.apps/obsidian/src/utils/utils.ts (1)
4-9: Useidas the unique identifier; drop anytypefallback
In apps/obsidian/src/types.ts, DiscourseNode only defines anidfield (notypeproperty), sonode.id === nodeTypeIdis correct—remove the suggestednode.typefallback.Likely an incorrect or invalid review comment.
apps/obsidian/src/utils/registerCommands.ts (1)
117-122: Wrap createCanvas callback in async try/catch and verify icon id
Replace in apps/obsidian/src/utils/registerCommands.ts:- callback: () => createCanvas(plugin), + callback: async () => { + try { + await createCanvas(plugin); + } catch (err) { + console.error("Failed to create canvas:", err); + // Optionally: new Notice("Failed to create canvas. See console for details."); + } + },Confirm that “layout-dashboard” is a valid Lucide icon in Obsidian.
trangdoan982
force-pushed
the
tldraw-obsidian
branch
from
47fd2e4 to
4254d5f
Compare
| @@ -0,0 +1,8 @@ | |||
| export const WHITE_LOGO_SVG = | |||
| '<svg width="18" height="18" viewBox="0 0 256 264" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M156.705 252.012C140.72 267.995 114.803 267.995 98.8183 252.012L11.9887 165.182C-3.99622 149.197 -3.99622 123.28 11.9886 107.296L55.4035 63.8807C63.3959 55.8881 76.3541 55.8881 84.3467 63.8807C92.3391 71.8731 92.3391 84.8313 84.3467 92.8239L69.8751 107.296C53.8901 123.28 53.8901 149.197 69.8751 165.182L113.29 208.596C121.282 216.589 134.241 216.589 142.233 208.596C150.225 200.604 150.225 187.646 142.233 179.653L127.761 165.182C111.777 149.197 111.777 123.28 127.761 107.296C143.746 91.3105 143.746 65.3939 127.761 49.4091L113.29 34.9375C105.297 26.9452 105.297 13.9868 113.29 5.99432C121.282 -1.99811 134.241 -1.99811 142.233 5.99434L243.533 107.296C259.519 123.28 259.519 149.197 243.533 165.182L156.705 252.012ZM200.119 121.767C192.127 113.775 179.168 113.775 171.176 121.767C163.184 129.76 163.184 142.718 171.176 150.71C179.168 158.703 192.127 158.703 200.119 150.71C208.112 142.718 208.112 129.76 200.119 121.767Z" fill="currentColor"/></svg>'; | |||
There was a problem hiding this comment.
This is to move the Roam svg to local so that it can be accessed offline as well
| }; | ||
|
|
||
| const TOOL_ARROW_ICON_DATA_URL = `data:image/svg+xml;base64,${btoa(TOOL_ARROW_ICON_SVG)}`; | ||
| const NODE_COLOR_ICON_DATA_URL = `data:image/svg+xml;base64,${btoa(NODE_COLOR_ICON_SVG)}`; |
There was a problem hiding this comment.
This is to move the Roam svg to local so that it can be accessed offline as well
There was a problem hiding this comment.
all the changes for the website will be merged earlier in PR #484, so you don't really have to review this part
| const nodeType = plugin.settings.nodeTypes.find( | ||
| (n) => n.id === pattern.nodeTypeId, | ||
| ); | ||
| const nodeType = getNodeTypeById(plugin, pattern.nodeTypeId); |
There was a problem hiding this comment.
convenient fix
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Just quickly scanned assuming there were no significant changes from the PR's that were merged into this.
Let's do a round of lint fixes (and a few other comments), then one last review/test before merging.
* cleaned * sm * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * 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 * 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 * pwd
* eng-658-add-existing-nodes-flow * address PR comments * small changes
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 -->
* add color setting * address PR reviews * address PR commens
* eng-812 : Update database cli tools: supabase, vercel, cucumber. * newer cucumber constrains node
* cleaned * sm * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
trangdoan982
force-pushed
the
tldraw-obsidian
branch
from
1a01259 to
e876042
Compare
|
Bravo, well done. 👊👏 This is a huge milestone! I'm excited for you to merge this 😁. |
Summary by CodeRabbit
New Features
Enhancements
Documentation
Chores
Style