[ENG-410] Node creation dialog #194
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 📝 WalkthroughWalkthroughA new modal component for node creation was introduced, enabling users to create nodes even without text selection. The command logic was updated to support this new modal, and a new command was added for direct node creation. Utility functions for node creation were refactored for improved separation of concerns. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ObsidianApp
participant CreateNodeModal
participant NodeTypeModal
participant createDiscourseNode
User->>ObsidianApp: Triggers "open-node-type-menu" command
ObsidianApp->>ObsidianApp: Checks if editor has selection
alt Selection exists
ObsidianApp->>NodeTypeModal: Open NodeTypeModal
else No selection
ObsidianApp->>CreateNodeModal: Open CreateNodeModal
CreateNodeModal->>User: User enters title and selects type
User->>CreateNodeModal: Confirms creation
CreateNodeModal->>createDiscourseNode: Create node with title and type
end
User->>ObsidianApp: Triggers "create-discourse-node" command
ObsidianApp->>CreateNodeModal: Open CreateNodeModal
CreateNodeModal->>User: User enters title and selects type
User->>CreateNodeModal: Confirms creation
CreateNodeModal->>createDiscourseNode: Create node with title and type
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 full review |
✅ Actions performedFull review 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: 0
🧹 Nitpick comments (2)
apps/obsidian/src/components/CreateNodeModal.tsx (1)
45-50: Consider adding Escape key support for better UX.The Enter key handling is well implemented. Consider adding Escape key support to cancel the modal.
this.titleInput.addEventListener("keydown", (e) => { if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); this.handleConfirm(); + } else if (e.key === "Escape") { + e.preventDefault(); + this.close(); } });apps/obsidian/src/utils/registerCommands.ts (1)
21-28: Consider extracting the common callback logic.The async callback is duplicated between both commands. Consider extracting it to a shared function for better maintainability.
+const createNodeCallback = (plugin: DiscourseGraphPlugin, editor: Editor) => + async (nodeType: DiscourseNode, title: string) => { + await createDiscourseNode({ + plugin, + nodeType, + text: title, + editor, + }); + }; // Then use it in both commands: -onNodeCreate: async (nodeType, title) => { - await createDiscourseNode({ - plugin, - nodeType, - text: title, - editor, - }); -}, +onNodeCreate: createNodeCallback(plugin, editor),Also applies to: 41-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/obsidian/src/components/CreateNodeModal.tsx(1 hunks)apps/obsidian/src/components/NodeTypeModal.tsx(1 hunks)apps/obsidian/src/index.ts(1 hunks)apps/obsidian/src/utils/createNode.ts(3 hunks)apps/obsidian/src/utils/registerCommands.ts(1 hunks)
🔇 Additional comments (9)
apps/obsidian/src/components/NodeTypeModal.tsx (1)
3-3:✅ Verification successful
LGTM! Verify the import path is correct.
The import path update aligns with the refactoring to separate text extraction from node creation logic.
🏁 Script executed:
#!/bin/bash # Verify that the processTextToDiscourseNode function exists in the new location ast-grep --pattern 'export function processTextToDiscourseNode($_) { $$$ }'Length of output: 77
🏁 Script executed:
#!/bin/bash # Search for the definition or export of processTextToDiscourseNode across the repository rg -n 'processTextToDiscourseNode' -g '*.ts' -g '*.tsx' .Length of output: 540
Confirmed:
processTextToDiscourseNodeexport is present inutils/createNode.tsI’ve verified that
processTextToDiscourseNodeis defined and exported inapps/obsidian/src/utils/createNode.ts(line 143), so the new import path is correct.LGTM!
apps/obsidian/src/index.ts (1)
7-7: LGTM! Import path updated correctly.Consistent with the refactoring pattern seen in other files.
apps/obsidian/src/components/CreateNodeModal.tsx (2)
113-134: Excellent error handling and validation.The async error handling is comprehensive, including proper type checking and user feedback through notices.
101-101: Good practice using setTimeout for focus.The setTimeout ensures the DOM is ready before focusing, which is a solid approach for modal components.
apps/obsidian/src/utils/registerCommands.ts (2)
13-32: Well-structured conditional logic for modal selection.The command correctly handles both scenarios - opening NodeTypeModal for text selection and CreateNodeModal for direct creation.
5-5:✅ Verification successful
Verify the createDiscourseNode function signature.
Ensure the new createDiscourseNode function accepts the parameters being passed and handles the editor parameter correctly when no text is selected.
🏁 Script executed:
#!/bin/bash # Verify the createDiscourseNode function signature and implementation ast-grep --pattern 'export function createDiscourseNode($params) { $$$ }'Length of output: 75
Locating and inspecting the
createNodeimplementation to verify thecreateDiscourseNodesignature:
🏁 Script executed:
#!/bin/bash # Find and display the start of the createNode source file file=$(fd createNode.ts -t f | head -n1) if [ -z "$file" ]; then echo "No createNode.ts file found" exit 1 fi echo "Found file: $file" sed -n '1,200p' "$file"Length of output: 4311
Inspecting registerCommands.ts usage of createDiscourseNode:
🏁 Script executed:
#!/bin/bash # Locate and display the top of registerCommands.ts to verify how createDiscourseNode is invoked file=$(fd registerCommands.ts -t f | head -n1) if [ -z "$file" ]; then echo "registerCommands.ts not found" exit 1 fi echo "Found file: $file" sed -n '1,200p' "$file"Length of output: 2327
createDiscourseNode signature matches its usage
The function is defined asexport const createDiscourseNode = async ({ plugin, nodeType, text, editor, }: { plugin: DiscourseGraphPlugin; nodeType: DiscourseNode; text: string; editor?: Editor; }) => …and in registerCommands.ts it’s invoked with exactly those properties. The optional editor parameter is handled correctly (replaceSelection only runs when editor is passed). No changes required.
apps/obsidian/src/utils/createNode.ts (3)
9-12: LGTM! Good improvement to text normalization.The parameter rename from
selectedTexttotextis more generic and appropriate. The text normalization logic correctly handles multiline input by replacing newlines with spaces and trimming whitespace, which is essential for creating valid node names.
110-141: Excellent refactoring to separate concerns.The function refactoring successfully separates text extraction from node creation logic:
- Takes
textdirectly instead of extracting from editor- Makes
editoroptional to support non-editor contexts (like modals)- Conditionally replaces selection only when both file creation succeeds and editor is provided
This design enables flexible node creation workflows while maintaining existing functionality.
143-160: Smart wrapper function maintains backward compatibility.The new
processTextToDiscourseNodefunction preserves the original API by extracting selected text from the editor and delegating tocreateDiscourseNode. This ensures existing code continues to work while enabling the new modal-based creation workflow.
trangdoan982
force-pushed
the
eng-409-choose-folder
branch
from
343ec81 to
884e55b
Compare
trangdoan982
force-pushed
the
eng-410-node-dialog
branch
from
c87ec4a to
e5ffe26
Compare
https://www.loom.com/share/912e32efc0ba4fceacfe322cbac9bce5
Summary by CodeRabbit
New Features
Improvements
Bug Fixes