feat: 添加大纲树节点删除功能 #1216
feat: 添加大纲树节点删除功能 #1216
Conversation
WalkthroughThe changes update the tree component in the UI by streamlining the eye icon rendering and adding a new delete icon. The two separate SVG elements for the eye condition are merged into one dynamic element based on the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Main.vue
participant Canvas as Canvas Module
participant H as History Module
U->>C: Mouseup on delete icon
C->>Canvas: Delete node and clear selection
C->>H: Record deletion in history
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[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 (1)
packages/plugins/tree/src/Main.vue (1)
136-144: Consider adding confirmation for node deletionThe implementation of the delete node functionality works well. However, since deletion is a destructive operation, consider adding a confirmation dialog to prevent accidental deletions of important nodes.
const delNode = (data) => { + const { message } = useMessage() const { clearSelect } = useCanvas().canvasApi.value - useCanvas().operateNode({ - type: 'delete', - id: data.id - }) - clearSelect() - useHistory().addHistory() + message.confirm({ + message: '确定要删除此节点吗？', + onConfirm: () => { + useCanvas().operateNode({ + type: 'delete', + id: data.id + }) + clearSelect() + useHistory().addHistory() + } + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/tree/src/Main.vue(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/plugins/tree/src/Main.vue (4)
30-31: UI enhancement: Added node deletion capability alongside visibility toggleGood implementation of the delete node functionality. The code now provides both visibility control (eye icon) and node deletion (delete icon) capabilities. The consolidated eye icon implementation is also a nice cleanup.
44-44: Added history tracking for node deletionGood addition of the useHistory import to enable undo/redo functionality for node deletion operations.
211-211: Properly exposed delNode function to the templateThe new function is correctly exposed in the setup return object.
246-256: UI behavior aligned between eye and delete iconsGood CSS implementation that ensures the delete icon follows the same visibility behavior as the eye icon - only appearing when hovering over a tree row. This creates a consistent user experience.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/plugins/tree/src/Main.vue (2)
31-31: Added delete functionality as requested in PRThe implementation adds a delete icon that calls the new
delNodemethod. This correctly implements the feature described in the PR objectives.Consider adding a confirmation dialog before deleting nodes, especially for nodes that might contain children or are critical to the page structure.
136-144: Well-structured node deletion implementationThe
delNodeimplementation correctly:
- Uses the canvas API for node deletion
- Clears selection after deletion to prevent UI issues
- Records the action in history for undo functionality
Consider adding:
- Error handling in case the deletion operation fails
- Validation to prevent deletion of essential nodes (if applicable)
const delNode = (data) => { const { clearSelect } = useCanvas().canvasApi.value + try { useCanvas().operateNode({ type: 'delete', id: data.id }) clearSelect() useHistory().addHistory() + } catch (error) { + console.error('Failed to delete node:', error) + // Consider adding a user notification here + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/tree/src/Main.vue(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (5)
packages/plugins/tree/src/Main.vue (5)
30-30: Code improvement - streamlined SVG icon implementationThe consolidation of two separate SVG icon elements into a single element with a dynamic name attribute is a good refactoring that reduces template complexity while maintaining the same functionality.
44-44: Good practice: Added history tracking for deletion actionsAdding the
useHistoryimport ensures that deletion operations can be undone, which is essential for a good user experience in any editing interface.
211-211: Correctly exposing the new methodThe
delNodemethod is properly exposed in the returned object, making it accessible to the template. This follows Vue 3 Composition API best practices.
246-248: Consistent styling for both iconsThe CSS selector now includes both the eye and delete icons, ensuring consistent visibility rules for both controls.
250-254: Consistent hover behavior for all controlsThe hover style now properly applies to both icons, creating a consistent user experience where all available actions become visible on hover.
1degrees
force-pushed
the
feat/delNodeInTree
branch
from
f038413 to
27845c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/plugins/tree/src/Main.vue (1)
136-144: Consider adding a confirmation dialog for deletionThe implementation of
delNodeis functional but could benefit from a confirmation step before deletion to prevent accidental deletions. Also, consider adding error handling to provide feedback if the deletion fails.const delNode = (data) => { const { clearSelect } = useCanvas().canvasApi.value - useCanvas().operateNode({ - type: 'delete', - id: data.id - }) - clearSelect() - useHistory().addHistory() + const { message } = useMessage() + message({ + type: 'confirm', + title: '确认删除', + message: `确定要删除节点 "${data.componentName}" 吗？`, + callback: () => { + try { + useCanvas().operateNode({ + type: 'delete', + id: data.id + }) + clearSelect() + useHistory().addHistory() + } catch (error) { + message({ + type: 'error', + message: `删除失败: ${error.message || '未知错误'}` + }) + } + } + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/tree/src/Main.vue(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/plugins/tree/src/Main.vue (4)
30-31: Nice refactoring of the eye icons and addition of deletion functionalityYou've effectively consolidated the eye icon implementation into a single conditional element and added a new delete icon. This is a good UI enhancement that keeps the interface consistent.
44-44: LGTM - Appropriate addition of useHistory importThe import of
useHistoryis correctly added to support the undo functionality for node deletion.
211-211: LGTM - Properly exposed the new methodGood job exposing the
delNodemethod in the returned object to make it available to the template.
246-248: LGTM - Consistent styling for both iconsThe CSS updates correctly maintain the same visibility behavior for both the eye and delete icons, showing them only on hover. This keeps the UI clean and consistent.
Also applies to: 250-254
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
2025-03-13.17.14.26.mov
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit