[ENG-350] Obsidian - Add template to node type setting #179
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughTemplate association functionality was added to discourse node types in the Obsidian Discourse Graph plugin. Users can now select a template for each node type via the UI, with templates sourced from the Obsidian Templates plugin. When creating new nodes, the selected template is applied to the node file. Documentation and utility functions were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NodeTypeSettings
participant TemplatesPlugin
participant Vault
User->>NodeTypeSettings: Open Node Type Settings
NodeTypeSettings->>TemplatesPlugin: Fetch available templates
TemplatesPlugin-->>NodeTypeSettings: Return template list
User->>NodeTypeSettings: Select template for node type
NodeTypeSettings->>Vault: Save node type with template selection
User->>App: Create new node of selected type
App->>Vault: Create new node file
App->>TemplatesPlugin: Apply selected template to new file
TemplatesPlugin->>Vault: Insert template content into file
App-->>User: Notify node creation (or warn if template fails)
Possibly related PRs
Suggested reviewers
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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/README.md (2)
53-57: Fix markdown indentation and improve language clarity.The template documentation content is excellent, but there are some formatting and language improvements needed:
Apply this diff to fix indentation and language issues:
- - **Template (Optional)**: Select a template from the dropdown to automatically apply template content when creating nodes of this type - - Templates are sourced from Obsidian's core Templates plugin - - Ensure you have the Templates plugin enabled and configured with a template folder - - The dropdown will show all available template files from your configured template folder +- **Template (Optional)**: Select a template from the dropdown to automatically apply template content when creating nodes of this type + - Templates are sourced from Obsidian's core Templates plugin + - Ensure you have the Templates plugin enabled and configured with a template folder + - The dropdown will show all available template files from your configured template folder🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
53-53: Unordered list indentation
Expected: 0; Actual: 3(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
64-71: Improve language clarity and fix missing articles.The template creation instructions are helpful but need language improvements:
Apply this diff to improve readability:
- - To create a new template: - -Create new folder to store templates +- To create a new template: + +Create a new folder to store templates -Specify template folder location in plugin settings menu +Specify the template folder location in the plugin settings menu🧰 Tools
🪛 LanguageTool
[uncategorized] ~64-~64: You might be missing the article “a” here.
Context: ... - To create a new template: Create new folder to store templates ![new folder...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~68-~68: You might be missing the article “the” here.
Context: ...g) Specify template folder location in plugin settings menu 
apps/obsidian/src/utils/templates.ts (1)
8-26: Consider improving type safety for internal API access.The function correctly handles the Templates plugin interaction, but accessing internal APIs through type casting poses risks for future compatibility.
Consider adding a more robust type check and potentially documenting the API dependency:
const getTemplatePluginInfo = (app: App): TemplatePluginInfo => { try { - const templatesPlugin = (app as any).internalPlugins?.plugins?.templates; + // @ts-ignore - Accessing Obsidian internal API for Templates plugin + const internalPlugins = app.internalPlugins; + const templatesPlugin = internalPlugins?.plugins?.templates; if (!templatesPlugin || !templatesPlugin.enabled) { return { isEnabled: false, folderPath: "" }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/obsidian/docs/media/choose-template.pngis excluded by!**/*.pngapps/obsidian/docs/media/create-template-file.pngis excluded by!**/*.pngapps/obsidian/docs/media/new-folder.pngis excluded by!**/*.pngapps/obsidian/docs/media/template.pngis excluded by!**/*.png
📒 Files selected for processing (6)
apps/obsidian/README.md(1 hunks)apps/obsidian/src/components/NodeTypeSettings.tsx(6 hunks)apps/obsidian/src/constants.ts(1 hunks)apps/obsidian/src/types.ts(1 hunks)apps/obsidian/src/utils/createNodeFromSelectedText.ts(2 hunks)apps/obsidian/src/utils/templates.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/obsidian/src/utils/createNodeFromSelectedText.ts (1)
apps/obsidian/src/utils/templates.ts (1)
applyTemplate(61-100)
apps/obsidian/src/components/NodeTypeSettings.tsx (1)
apps/obsidian/src/utils/templates.ts (1)
getTemplateFiles(28-59)
🪛 LanguageTool
apps/obsidian/README.md
[uncategorized] ~64-~64: You might be missing the article “a” here.
Context: ... - To create a new template: Create new folder to store templates ![new folder...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~68-~68: You might be missing the article “the” here.
Context: ...g) Specify template folder location in plugin settings menu 
🪛 markdownlint-cli2 (0.17.2)
apps/obsidian/README.md
53-53: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
🔇 Additional comments (10)
apps/obsidian/src/types.ts (1)
5-5: LGTM: Clean type definition for template support.The optional
templateproperty addition is well-defined and maintains backward compatibility. The string type is appropriate for template names.apps/obsidian/src/constants.ts (1)
9-9: LGTM: Consistent template initialization.The empty string initialization for the
templateproperty across all default node types is appropriate and maintains consistency. This provides a clean default state for the new template functionality.Also applies to: 15-15, 21-21
apps/obsidian/src/utils/createNodeFromSelectedText.ts (2)
5-5: LGTM: Proper import for template functionality.The import statement correctly brings in the
applyTemplatefunction needed for the new template feature.
46-58: LGTM: Well-implemented template application with good error handling.The template application logic is well-structured:
- Properly validates template existence and non-empty content
- Correctly placed after file creation and front matter processing
- Provides user feedback on template application failures without breaking the node creation flow
- Uses appropriate async/await pattern
The error handling strikes the right balance by showing a warning notice rather than failing the entire operation, allowing users to manually apply templates if needed.
apps/obsidian/README.md (1)
53-72: LGTM: Comprehensive template documentation with clear instructions.The documentation provides excellent coverage of the template functionality:
- Clear explanation of the optional template feature
- Proper prerequisites (Templates plugin requirement)
- Step-by-step instructions for both using and creating templates
- Good visual aids to support the instructions
The content accurately describes the new template integration and will help users effectively utilize this feature.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~64-~64: You might be missing the article “a” here.
Context: ... - To create a new template: Create new folder to store templates ![new folder...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~68-~68: You might be missing the article “the” here.
Context: ...g) Specify template folder location in plugin settings menu 
🪛 markdownlint-cli2 (0.17.2)
53-53: Unordered list indentation
Expected: 0; Actual: 3(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
apps/obsidian/src/components/NodeTypeSettings.tsx (4)
1-1: LGTM: Clean import additions.The imports are correctly added and follow the existing patterns in the file.
Also applies to: 12-12
21-21: LGTM: Proper state management and async loading.The template files state and useEffect implementation correctly handles the asynchronous loading of templates. The dependency array properly includes
plugin.appto ensure re-loading if the app instance changes.Also applies to: 23-29
57-62: LGTM: Consistent template property initialization.The template property is correctly initialized as an empty string in both the node type creation scenarios, maintaining consistency with the type definition.
Also applies to: 87-87
172-185: LGTM: Well-implemented template selection UI.The select dropdown provides a clean interface for template selection with:
- Proper value binding and change handling
- "No template" option for clearing selection
- Dynamic population from loaded template files
- Consistent styling with other form elements
apps/obsidian/src/utils/templates.ts (1)
28-59: LGTM: Robust template file discovery with proper error handling.The function correctly:
- Validates plugin state and configuration
- Safely checks folder existence and type
- Filters for markdown files only
- Returns sorted results
- Handles errors gracefully
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Nice.
Only blocking comments are the async function and named params.
Going to approve this so you can merge when complete.
Also, might be worth creating a task to enhance the UX when a user is trying to set a template but doesn't have any templates defined / template plugin is disabled. I know it's in the readme, but right in the UI somewhere would be ideal.
We could disable the dropdown and have a message on hover, for example.
|
|
||
| const currentContent = await app.vault.read(targetFile); | ||
|
|
||
| const newContent = currentContent + templateContent; |
There was a problem hiding this comment.
Let's bring this up with the UX team to see if this is the expected behavior.
https://www.loom.com/share/9a5c3b09a6a4458d8592da767af17262
Summary by CodeRabbit
New Features
Documentation