[ENG-871] Convert text and image into a discourse node (Obsidian) #523
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughIntroduces a context menu for Obsidian's canvas that enables converting canvas shapes to Discourse nodes. Adds a CustomContextMenu component wired into TldrawViewComponent, a convertToDiscourseNode utility orchestrating shape-to-node conversion for text and image types, and updates TypeScript configuration to include TSX files. Changes
Sequence DiagramsequenceDiagram
participant User
participant Canvas as Canvas Context Menu
participant Converter as convertToDiscourseNode
participant Editor
participant FS as Disk/Settings
User->>Canvas: Right-click shape
Canvas->>Canvas: Check shape type
alt Text or Image Shape
Canvas->>Canvas: Show "Convert To" menu
User->>Canvas: Select node type
Canvas->>Converter: convertToDiscourseNode(shape, nodeType)
else Other Shape
Canvas->>User: No conversion option
end
rect rgb(200, 220, 255)
note over Converter: Text Shape Flow
Converter->>Converter: Render plaintext
Converter->>FS: Create node file
Converter->>Editor: Create discourse node shape
end
rect rgb(220, 200, 255)
note over Converter: Image Shape Flow
Converter->>Converter: Get image from shape
Converter->>Canvas: Open node type modal
Converter->>FS: Create node file
Converter->>FS: Embed image in node
Converter->>Editor: Create discourse node shape
end
rect rgb(255, 240, 200)
note over Converter: Error Handling
Converter->>Converter: Try/catch conversion
Converter->>User: Show error toast on failure
end
Converter->>User: Show success toast
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/obsidian/src/components/canvas/CustomContextMenu.tsx (2)
33-35: Remove redundant optional chaining.Since
selectedShapeis already checked for truthiness in the condition, the optional chaining on line 35 (selectedShape?.type) is unnecessary.Apply this diff:
const shouldShowConvertTo = selectedShape && - (selectedShape?.type === "text" || selectedShape?.type === "image"); + (selectedShape.type === "text" || selectedShape.type === "image");
43-59: Prefer template literal for string concatenation.Line 47 uses string concatenation instead of a template literal. For consistency with modern JavaScript practices, consider using a template literal.
Apply this diff:
<TldrawUiMenuItem key={nodeType.id} id={`convert-to-${nodeType.id}`} - label={"Convert to " + nodeType.name} + label={`Convert to ${nodeType.name}`} icon="file-type" onSelect={() => { void convertToDiscourseNode({ editor, shape: selectedShape, nodeType, plugin, canvasFile, }); }} />apps/obsidian/src/components/canvas/utils/convertToDiscourseNode.ts (1)
265-279: Clarify intentional spacing in template literal.Line 277 contains a template literal with what appears to be extra whitespace:
`${data}\n${imageEmbed} \n`. The space before the final\nmay be intentional for formatting, but it looks unusual. Consider clarifying or removing it for consistency.If the space is not intentional, apply this diff:
await plugin.app.vault.process(nodeFile, (data: string) => { - return `${data}\n${imageEmbed} \n`; + return `${data}\n${imageEmbed}\n`; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/obsidian/src/components/canvas/CustomContextMenu.tsx(1 hunks)apps/obsidian/src/components/canvas/TldrawViewComponent.tsx(2 hunks)apps/obsidian/src/components/canvas/utils/convertToDiscourseNode.ts(1 hunks)apps/obsidian/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Prefertypeoverinterface
Use explicit return types for functions
Avoidanytypes when possible
Files:
apps/obsidian/src/components/canvas/TldrawViewComponent.tsxapps/obsidian/src/components/canvas/CustomContextMenu.tsxapps/obsidian/src/components/canvas/utils/convertToDiscourseNode.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx,js,jsx}: Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use Prettier with the project's configuration
Maintain consistent naming conventions: PascalCase for components and types
Maintain consistent naming conventions: camelCase for variables and functions
Maintain consistent naming conventions: UPPERCASE for constants
Files:
apps/obsidian/src/components/canvas/TldrawViewComponent.tsxapps/obsidian/src/components/canvas/CustomContextMenu.tsxapps/obsidian/src/components/canvas/utils/convertToDiscourseNode.ts
apps/obsidian/**
📄 CodeRabbit inference engine (.cursor/rules/obsidian.mdc)
Use the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for all code in the Obsidian plugin
Files:
apps/obsidian/src/components/canvas/TldrawViewComponent.tsxapps/obsidian/src/components/canvas/CustomContextMenu.tsxapps/obsidian/src/components/canvas/utils/convertToDiscourseNode.tsapps/obsidian/tsconfig.json
🧠 Learnings (1)
📚 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
🔇 Additional comments (10)
apps/obsidian/tsconfig.json (1)
3-3: LGTM!The addition of TSX file support is necessary for the new React components introduced in this PR.
apps/obsidian/src/components/canvas/TldrawViewComponent.tsx (2)
53-53: LGTM!The import is correctly structured and uses the appropriate relative path.
341-344: Verify the necessity of the ESLint disable comment.The
eslint-disablecomment for@typescript-eslint/naming-conventionmay be needed due to Tldraw's component override API expectations, but it's worth confirming whether this is truly necessary or if there's an alternative approach that doesn't require disabling the linting rule.apps/obsidian/src/components/canvas/CustomContextMenu.tsx (2)
1-18: LGTM!Imports are well-organized, and the type definition follows the project's coding guidelines (using
typeoverinterfaceand proper naming conventions).
20-31: LGTM!Component follows the coding guidelines: uses arrow function, PascalCase naming, and properly leverages React hooks for reactive state management.
apps/obsidian/src/components/canvas/utils/convertToDiscourseNode.ts (5)
1-28: LGTM!Imports are well-organized, and the type definition adheres to the coding guidelines (using
typeoverinterfaceand PascalCase naming).
30-57: LGTM!The main conversion function follows coding guidelines with an explicit return type, uses an arrow function, and implements proper error handling with appropriate user feedback.
111-173: Verify return value behavior for image conversion.The function returns
shapeIdimmediately after opening the modal (line 172), which means it will always returnundefinedsince the modal interaction is asynchronous andonNodeCreatehasn't executed yet. Verify whether this non-blocking behavior is intentional, as callers won't receive the actual shape ID of the created discourse node.If the return value is expected to provide the created shape ID, consider using a Promise that resolves when the modal completes:
return new Promise<TLShapeId | undefined>((resolve) => { const modal = new CreateNodeModal(plugin.app, { nodeTypes: plugin.settings.nodeTypes, plugin, initialNodeType: nodeType, initialTitle: "", onNodeCreate: async (selectedNodeType: DiscourseNode, title: string) => { try { // ... existing logic ... resolve(shapeId); } catch (error) { console.error("Error creating node from image:", error); resolve(undefined); throw error; } }, }); modal.open(); });
175-222: LGTM!The function follows coding guidelines with explicit types, proper parameter destructuring for multiple parameters, and includes appropriate error handling. The minimum dimension enforcement and history management are well-implemented.
224-264: LGTM!The
getImageFileFromShapehelper includes proper error handling and returnsnullfor all error cases, providing a safe fallback for the caller.
apps/obsidian/tsconfig.json
Outdated
| { | ||
| "extends": "@repo/typescript-config/react-library.json", | ||
| "include": ["**/*.ts"], | ||
| "include": ["**/*.ts", "**/*.tsx"], |
trangdoan982
deleted the
eng-871-convert-text-image-into-a-discourse-node
branch
https://www.loom.com/share/4e1d2489760b439f80734e7c43eccd9a
Summary by CodeRabbit