feat: multi-select node dragging function #1245
Conversation
WalkthroughThe changes introduce multi-drag functionality to the canvas interface by adding a new component, Changes
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:
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
🔭 Outside diff range comments (1)
packages/canvas/container/src/CanvasContainer.vue (1)
1-431: 💡 Verification agent🧩 Analysis chain
Verify if tests have been added for multi-drag functionality.
The PR description notes that tests have not been added for the changes. Consider adding tests to ensure the multi-drag functionality works as expected in various scenarios.
🏁 Script executed:
#!/bin/bash # Look for test files that might test the multi-drag functionality echo "Searching for potential test files..." fd -t f -e spec.js -e spec.ts -e test.js -e test.ts 'drag|select|multi|canvas' packages/canvas/Length of output: 182
Action Required: Add Tests for Multi-Drag Functionality
Our search in the
packages/canvas/directory did not return any test files related to drag, select, or multi functionalities. Since no tests were found verifying the multi-drag behavior, please add appropriate tests to cover various multi-drag scenarios and ensure the robustness of this functionality.
🧹 Nitpick comments (4)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (2)
4-7: Consider adding an ARIA announcement for accessibility.
When displaying “正在拖拽 {{ multiStateLength }} 个组件” (line 5), you could provide anaria-liveregion or similar mechanism to announce changes to screen readers.
17-34: Consider optimizing preview rendering for large selections.
Displaying multiple preview items can grow overhead if users select numerous nodes. You might consider a virtualization approach or a limit on displayed previews if performance becomes an issue.packages/canvas/container/src/composables/useMultiSelect.ts (1)
25-26: Evaluate the UX of removing selection only when the mouse is not pressed.
This approach can prevent unintended removal during drag, but verify it doesn’t introduce confusion if users expect the item to be deselected immediately.packages/canvas/container/src/composables/useMultiDrag.ts (1)
526-683: Refactor the insertion of multiple nodes to handle edge concurrency.
Though the approach of removing nodes first and then re-inserting them in bulk is sensible, consider concurrent modifications if other operations might occur within the same timeframe. Locking or a synchronized approach might be beneficial in real-time multi-user scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)packages/canvas/container/src/components/CanvasAction.vue(3 hunks)packages/canvas/container/src/components/CanvasMultiDragIndicator.vue(1 hunks)packages/canvas/container/src/composables/useMultiDrag.ts(1 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(3 hunks)packages/canvas/container/src/container.ts(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/canvas/container/src/composables/useMultiDrag.ts (1)
packages/canvas/container/src/container.ts (15)
PositionType(29-29)querySelectById(352-362)POSITION(31-38)lineState(121-123)allowInsert(464-483)isAncestor(485-500)dragState(84-86)initialDragState(40-48)getElement(215-237)getDocument(65-65)getSchema(81-81)removeNode(319-324)insertNode(810-840)getController(63-63)syncNodeScroll(657-660)
🔇 Additional comments (26)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (2)
9-15: Verify thatlineState.forbiddencovers all edge cases.
If the user drags over a forbidden area, double-check whether the UI accurately reflects this state in all scenarios (e.g., partial overlaps).
63-265: Great use of scoped styles and transitions!
The styling seems well-structured with relevant animation keyframes. This is readable and consistent with the codebase's theme usage.packages/canvas/container/src/composables/useMultiSelect.ts (1)
8-9: Confirm thatisMouseDownis appropriately toggled in all mouse event handlers.
EnsureisMouseDown.valueis set totrueon mousedown and reset on mouseup so the multi-selection logic accurately reflects user interactions.packages/canvas/container/src/components/CanvasAction.vue (3)
4-4: Dynamic class bindings are well-defined.
Conditionally applying'multi-select'and'dragging'is straightforward and should maintain clarity for multi-drag states.
192-195: EnsureisMultiDraggingprop is updated correctly.
Confirm thatisMultiDraggingaligns with the composable’sisMultiDragging()for consistent UI feedback.
723-749: Styling for.multi-select.draggingis solid.
The lowered opacity, dashed border, and pulsing animation effectively communicate multi-drag. This is a good UX improvement.packages/canvas/container/src/composables/useMultiDrag.ts (2)
99-130: Multi-drag initialization looks correct.
Storing initial mouse positions and offset references for each selected node is clear and avoids unnecessary overhead.
251-390: Check correctness of body-drop edge cases.
The logic handles dropping onto “body” or empty space by scanning body’s children. Please confirm it behaves correctly for nested structures or for pages without children.packages/canvas/container/src/container.ts (4)
29-39: Good addition of type safety for position constants.The introduction of the
PositionTypetype alias and the refactoring ofPOSITIONconstant withObject.freeze()and proper type assertions enhances type safety and improves code maintainability.
40-40: Exporting initialDragState is appropriate.Making the
initialDragStateexportable allows it to be reused by other components, particularly the new multi-drag functionality.
485-500: Good decision to export isAncestor function.Exporting the
isAncestorfunction makes it available for use in the multi-drag functionality. This function checks parent-child relationships between nodes, which is essential for validating drag operations.
810-810: Enhanced type safety for insertNode function.Adding the
PositionTypetype annotation to thepositionparameter improves type checking and helps prevent invalid position values from being passed to the function.packages/canvas/container/src/CanvasContainer.vue (14)
11-11: Good addition of isMultiDragging prop to CanvasAction.Adding the
isMultiDraggingprop to the CanvasAction component allows it to adjust its behavior based on whether multiple nodes are being dragged.
16-22: Well-structured multi-drag indicator component.The new
canvas-multi-drag-indicatorcomponent is properly configured with all necessary props to display the state of multi-drag operations, enhancing the user experience.
73-75: Appropriate imports for new functionality.Adding imports for
CanvasMultiDragIndicatorcomponent anduseMultiDragcomposable correctly sets up the dependencies for the multi-drag feature.
106-106: Registered new component in components list.Properly registering the
CanvasMultiDragIndicatorcomponent in the components list ensures it's available for use in the template.Also applies to: 107-107
127-127: Good extraction of isMouseDown state.Extracting the
isMouseDownstate from theuseMultiSelectcomposable enables better tracking of mouse state, which is essential for multi-drag operations.
130-131: Well-structured use of useMultiDrag composable.The destructuring assignment from the
useMultiDragcomposable extracts all necessary functions and state for implementing the multi-drag functionality.
145-172: Improved event handling logic in setCurrentNode.The modifications to
setCurrentNodecorrectly prioritize handling of right-click menu and multi-drag start over single-node dragging, improving user experience.
174-180: Clear separation of single-node dragging logic.The single-node dragging logic is now properly separated and only executed when multi-drag is not activated, ensuring the correct behavior in both scenarios.
250-251: Mouse state tracking added to mousedown event.Setting
isMouseDown.value = truein the mousedown event handler correctly tracks the mouse state, which is essential for multi-drag operations.
265-274: Prioritized multi-drag in mousemove event.The mousemove event handler now correctly prioritizes multi-drag moving over normal dragging, ensuring a smooth user experience when dragging multiple nodes.
276-289: Enhanced mouseup event handling.The mouseup event handler now updates the
isMouseDownstate and prioritizes multi-drag ending over normal mouseup, ensuring proper cleanup of drag state.
295-298: Prioritized multi-drag in dragover event.The dragover event handler now correctly prioritizes multi-drag moving, ensuring a consistent experience across different drag initiation methods.
304-307: Prioritized multi-drag in drop event.The drop event handler now correctly prioritizes multi-drag ending, ensuring proper cleanup of drag state regardless of how the drag was initiated.
396-398: Exposed necessary functions and state for the template.Exporting
isMultiDragging,multiDragState, andgetMultiDragPositionTextmakes them available for use in the template, particularly for the new multi-drag indicator component.
SonyLeo
force-pushed
the
feat/multi-node-drag
branch
from
5c0a841 to
96d3040
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (3)
4-7: Consider externalizing status text for i18n flexibility
Currently, the text "正在拖拽 {{ multiStateLength }} 个组件" is hardcoded in Chinese, which may limit internationalization options if the application needs multi-language support.
38-60: Check prop defaults and usage
All props (e.g.,lineState,multiDragState,getMultiDragPositionText) default to empty objects or strings. Verify that these defaults align with expected real usage, ensuring no references produce undefined property errors at runtime.
63-265: Recommend auditing style complexity
The advanced styling and animations are thorough and visually appealing. However, consider centralizing shared constants (e.g., color or spacing) or using theme variables to keep the styles maintainable and consistent across the codebase.packages/canvas/container/src/components/CanvasAction.vue (2)
192-195: Confirm newisMultiDraggingprop is correctly set
The propisMultiDraggingdefaults to false. Validate that upstream logic properly toggles this prop to reflect the real multi-drag status throughout the drag lifecycle.
752-774: Check animation performance
Thepulse-borderandpulse-bgkeyframes run continuously. For large-scale usage (e.g., dozens of selected items), consider potential performance effects and use debouncing or conditional animations.packages/canvas/container/src/container.ts (1)
40-48: ConfirminitialDragStatealigns with multi-drag logic
The newdraging,data, andpositionfields may need to account for multi-drag scenarios if multiple items are being dragged. Validate that higher-level logic does not overlook these cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)packages/canvas/container/src/components/CanvasAction.vue(3 hunks)packages/canvas/container/src/components/CanvasMultiDragIndicator.vue(1 hunks)packages/canvas/container/src/composables/useMultiDrag.ts(1 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(3 hunks)packages/canvas/container/src/container.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/canvas/container/src/composables/useMultiSelect.ts
- packages/canvas/container/src/composables/useMultiDrag.ts
- packages/canvas/container/src/CanvasContainer.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (7)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (2)
17-31: Validate dragged node data inmultiDragState
When rendering multiple previews, each draggednodeis assumed to haveid,componentName, and optionalprops. Ensure upstream code consistently sets these properties to prevent potential errors when a node is missing expected fields.
9-15:Details
✅ Verification successful
Ensure line state object is always valid
The conditional checklineState.height && lineState.widtheffectively prevents rendering an empty drop indicator. However, to avoid potential runtime errors, confirmlineStateis not null or undefined in all possible usage scenarios.Run a quick check across the codebase to confirm
lineStateis never assigned null or undefined by searching for its assignments:
🏁 Script executed:
#!/bin/bash rg -A 3 "lineState\s*="Length of output: 973
Review Verification:
lineStateValidationI verified the assignments across the codebase and confirmed that
lineStateis always created as a reactive object (inpackages/canvas/container/src/container.ts) and consistently passed to components (e.g., inCanvasContainer.vue). There’s no evidence of it ever being set to null or undefined. The current conditional (lineState.height && lineState.width) is therefore sufficient to guard against rendering an empty drop indicator.packages/canvas/container/src/components/CanvasAction.vue (2)
4-4: Validate dynamic class binding
Ensuring[ 'multi-select': multiStateLength > 1 ]and{ dragging: isMultiDragging }logic matches the multi-drag state is crucial for clear visual feedback. Double-check thatmultiStateLengthcorrectly updates from your multi-drag feature to avoid mismatch with the actual dragging state.
723-750: Ensure consistent multi-select styling
The.multi-selectclass shift to a dashed border and partial opacity is visually clear. Verify that each multi-selected element remains selectable or visible when multiple items overlap, preventing user confusion during multi-drag.packages/canvas/container/src/container.ts (3)
29-38: Enhanced type safety for position management
DefiningPositionTypeand freezingPOSITIONhelps avoid accidental modification of position constants. This is a good practice for clarity and reliability.
485-485: ExportingisAncestor
Exporting this function broadens its applicability. Ensure that only valid consumers call it, as misuse may cause unexpected recursion or misinterpretation if the hierarchy is not properly set up.
810-810: Overlay definition ofinsertNode
Theposition: PositionType = POSITION.INsignature enforces consistent use ofPOSITION. Great approach to strongly-type insertion positions and reduce accidental usage of invalid strings.
hexqi
left a comment
hexqi
left a comment
There was a problem hiding this comment.
似乎还是单个节点的拖拽
选中文本3和4拖拽：
拖拽后：
SonyLeo
force-pushed
the
feat/multi-node-drag
branch
from
96d3040 to
fff1549
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (1)
3-7: Consider internationalization for user-facing text.The component uses hardcoded Chinese text which may cause issues for non-Chinese users. Consider using an i18n system for better localization support.
- <span>正在拖拽 {{ multiStateLength }} 个组件</span> + <span>{{ i18n.t('canvas.dragging_components', { count: multiStateLength }) }}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)packages/canvas/container/src/components/CanvasAction.vue(3 hunks)packages/canvas/container/src/components/CanvasMultiDragIndicator.vue(1 hunks)packages/canvas/container/src/composables/useMultiDrag.ts(1 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(3 hunks)packages/canvas/container/src/container.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/canvas/container/src/composables/useMultiSelect.ts
- packages/canvas/container/src/components/CanvasAction.vue
- packages/canvas/container/src/container.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (31)
packages/canvas/container/src/components/CanvasMultiDragIndicator.vue (6)
1-35: Well-structured template with clear section organization.The template is organized into three logical sections:
- Multi-drag status indicator (lines 4-7)
- Drop position indicator (lines 9-15)
- Node preview container (lines 17-34)
This organization makes the component's purpose clear and maintains good separation of concerns.
38-61: Props structure is clean and well-typed.The component has a clear API with well-defined props and default values. Each prop has an appropriate type definition which enhances type safety.
63-96: Good use of CSS variables for theming.The component leverages CSS variables like
--te-canvas-container-bg-color-checkedfor theming, which allows for consistent styling across the application and easier customization.
97-172: Well-implemented drop indicators with animation effects.The drop indicator styles provide clear visual feedback with different positions (top, left, bottom, right, in) and states (normal, forbidden). The pulse animations enhance the user experience by drawing attention to the active drop zone.
173-228: Excellent preview implementation with stacked items.The multi-drag preview provides a stacked view of dragged items with:
- Sequential numbering
- Component name display
- Optional ID display
- Z-index management for proper stacking
- Hover effects for better interaction
This gives users clear feedback about what they're dragging.
230-264: Well-defined animations for visual feedback.The keyframe animations (
preview-pulse,pulse-drop-line,pulse-drop-border) provide subtle but effective visual cues during the drag operation, making the interaction more intuitive.packages/canvas/container/src/CanvasContainer.vue (14)
11-11: Good integration of multi-drag state with CanvasAction component.The
isMultiDragging()prop is correctly passed to the CanvasAction component, allowing it to adapt its appearance based on the multi-drag state.
16-22: Well-implemented multi-drag indicator integration.The new CanvasMultiDragIndicator component is properly integrated with all necessary props:
- lineState for positioning
- multiDragState for node information
- multiStateLength for count display
- isMultiDragging for conditional rendering
- getMultiDragPositionText for position information
This ensures the indicator has all the data it needs to provide appropriate visual feedback.
73-75: Good component and composable imports.The necessary imports are added for the new multi-drag functionality: the CanvasMultiDragIndicator component and the useMultiDrag composable.
105-106: Component registration is correctly updated.The new CanvasMultiDragIndicator component is properly registered in the components object.
127-131: Clean composable integration with destructuring.The code uses destructuring to extract both the multiSelectedStates and isMouseDown from useMultiSelect, as well as all necessary methods and state from useMultiDrag. This makes the code more readable and maintainable.
146-152: Improved event handling with clear priority.The event handling logic has been restructured to have a clear order of operations:
- Early return if no element
- Handle right-click menu first
- Then handle drag operations
This approach prevents conflicts between different interactions.
169-172: Multi-drag operation takes precedence over single-node drag.The code correctly prioritizes multi-drag handling before single-node drag operations, which ensures consistent behavior when multiple nodes are selected.
173-180: Single-node drag behavior is preserved with proper condition.The single-node drag operation is only triggered when:
- The left mouse button is pressed
- The target is not the body element
- Only one node is selected
This ensures compatibility with the existing drag behavior while adding multi-drag functionality.
250-252: Mouse down state tracking added for multi-select behavior.The isMouseDown state is now tracked in the mousedown event, which is important for coordinating multi-select and multi-drag operations.
265-274: Mouse move handler prioritizes multi-drag operations.The mousemove event handler first attempts to handle multi-drag movement with
moveMultiDrag(), falling back to the standarddragMove()if the multi-drag handler returns false. This ensures proper event handling precedence.
277-288: Clean mouse up event handling with state management.The mouseup event handler:
- Updates the
isMouseDownstate- Attempts to handle multi-drag end first
- Falls back to standard drag end handling if necessary
This approach maintains a clear separation of concerns and proper event handling precedence.
295-298: Consistent drag prioritization in dragover event.The dragover event handler follows the same pattern as mousemove, prioritizing multi-drag operations before standard drag operations.
304-307: Consistent drop prioritization in drop event.The drop event handler follows the same pattern as mouseup, prioritizing multi-drag end before standard drag end handling.
374-398: Exports include all necessary multi-drag state and methods.The setup function returns all the necessary multi-drag related state and methods:
- isMouseDown for tracking mouse state
- isMultiDragging for conditional rendering
- multiDragState for node information
- getMultiDragPositionText for position description
This makes them available to the template and child components.
packages/canvas/container/src/composables/useMultiDrag.ts (11)
24-65: Well-defined TypeScript interfaces for type safety.The code defines clear interfaces for:
- Position (x, y coordinates)
- Offset (drag offset data)
- NodeSchema (component data structure)
- MultiDragState (drag operation state)
- SelectState (selection state)
This provides good type safety and makes the code more maintainable.
77-87: Clean initial state with appropriate defaults.The initialMultiDragState object provides sensible defaults for all properties:
- Boolean flags initialized to false
- Collections initialized to empty
- Object references initialized to null
This prevents undefined behavior when the state is first used.
98-130: Well-implemented drag start handler with validation.The startMultiDrag function:
- Checks if multi-selection is active (>1 node selected)
- Verifies the clicked node is part of the selection
- Records initial mouse position and target node
- Calculates and stores offset for each node
This ensures multi-drag operations only start when appropriate.
133-159: Precise drop position calculation logic.The calculateDropPosition function implements sophisticated logic to determine the drop position:
- Uses proportional thresholds based on element size (min(20, rect.height/3))
- Prioritizes edge detection (top, bottom, left, right)
- Falls back to container insertion (IN) for containers
- Defaults to bottom position as a last resort
This provides accurate and intuitive drop positioning.
170-248: Comprehensive validation for drop operations.The checkAllowInsert function performs thorough validation:
- Special handling for body element
- Prevents circular references (dropping into descendants)
- Prevents invalid parent-child relationships
- Validates against component-specific insertion rules
This ensures data integrity is maintained during drag operations.
251-274: Drag threshold implementation prevents accidental drags.The moveMultiDrag function includes a drag threshold check:
- Calculates distance moved from initial position
- Only starts dragging if threshold (5px) is exceeded
- Clears single-drag state when multi-drag starts
This creates a more deliberate drag interaction and prevents accidental movements.
285-390: Special handling for body and edge cases.The code includes detailed handling for special cases:
- Dragging over the document body
- Finding the closest node when dragging between elements
- Handling empty body scenarios
- Setting appropriate forbidden states when needed
This ensures the drag operation works correctly in all contexts.
404-472: Self-drag prevention with sibling target adjustment.The code detects and handles self-dragging scenarios:
- Identifies when dragging over a selected node
- Adjusts target to nearest sibling when appropriate
- Updates lineState to reflect the adjusted target
- Falls back to forbidden state when no valid target is found
This prevents invalid operations like dragging a node onto itself.
525-683: Batch operation for node insertion and history management.The endMultiDrag function:
- Collects all valid operations into a batch
- Removes source nodes first to prevent conflicts
- Inserts nodes at their target positions
- Adds a single history entry for the entire operation
- Rebuilds the multi-selection state after insertion
This approach maintains schema integrity and provides clean undo/redo support.
686-725: Helpful position text generation for UI feedback.The getMultiDragPositionText computed property:
- Provides clear descriptions for each position type
- Includes the target component name for context
- Shows appropriate messages for forbidden drops
- Updates reactively as the drag state changes
This enhances the user experience with informative feedback.
727-735: Clean API export with all necessary functions and state.The composable exports a well-defined API with:
- State object (multiDragState)
- Helper for position text (getMultiDragPositionText)
- Lifecycle methods (startMultiDrag, moveMultiDrag, endMultiDrag)
- State check helper (isMultiDragging)
This provides everything needed to integrate the functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/canvas/container/src/composables/useMultiDrag.ts (3)
169-248: 🛠️ Refactor suggestionRefactor complex method into smaller, focused functions.
The
checkAllowInsertmethod is too long (80 lines) and handles multiple responsibilities, making it difficult to maintain and test. Break it into smaller, focused functions for each specific check.// Main function to check if insertion is allowed const checkAllowInsert = ( configure: { isContainer?: boolean } | null, nodes: NodeSchema[], targetId: string, position: PositionType ): boolean => { // 如果没有配置，不允许放置 if (!configure) return false // 获取目标节点的信息 const { parent: targetParent } = useCanvas().getNodeWithParentById(targetId) || {} const targetParentId = targetParent?.id // 如果目标是body，特殊处理 if (targetId === 'body') { return checkBodyInsert(position, nodes); } // 如果目标节点的父节点是body，特殊处理 if (targetParentId === 'body' && (position === POSITION.TOP || position === POSITION.BOTTOM)) { return checkBodyChildInsert(nodes); } // 检查所有节点是否都允许放置 return checkNodesInsert(nodes, targetId, targetParentId, position, configure); } // Helper function to check insertion in body const checkBodyInsert = (position: PositionType, nodes: NodeSchema[]): boolean => { // 对于body，允许放置到内部、上方和下方 if (position !== POSITION.IN && position !== POSITION.TOP && position !== POSITION.BOTTOM) { // 强制将position设置为IN，因为body只能放置到内部、上方或下方 lineState.position = POSITION.IN; } // 检查所有节点是否都允许放置到body内 for (const node of nodes) { if (!allowInsert({ isContainer: true }, node)) { return false; } } return true; } // Helper function to check insertion for body's direct children const checkBodyChildInsert = (nodes: NodeSchema[]): boolean => { // 检查所有节点是否都允许放置到body内 for (const node of nodes) { if (!allowInsert({ isContainer: true }, node)) { return false; } } return true; } // Helper function to check if nodes can be inserted at target position const checkNodesInsert = ( nodes: NodeSchema[], targetId: string, targetParentId: string | undefined, position: PositionType, configure: { isContainer?: boolean } | null ): boolean => { for (const node of nodes) { // 如果是放置到容器内，检查节点是否是目标节点的祖先 if (position === POSITION.IN && isAncestor(node.id, targetId)) { return false; } // 如果是放置到节点前后，检查节点是否是目标节点的父节点 if ( (position === POSITION.TOP || position === POSITION.BOTTOM || position === POSITION.LEFT || position === POSITION.RIGHT) && node.id === targetParentId ) { return false; } // 检查节点是否允许放置到目标位置 if (position === POSITION.IN) { // 放置到容器内需要检查容器的配置 if (!allowInsert(configure, node)) { return false; } } else { // 放置到节点前后需要检查父节点的配置 const { parent: targetParent } = useCanvas().getNodeWithParentById(targetId) || {}; const parentConfigure = targetParent ? getConfigure(targetParent.componentName) : { isContainer: true }; if (!allowInsert(parentConfigure, node)) { return false; } } } return true; }
250-523:⚠️ Potential issueRefactor overly complex
moveMultiDragfunction.This function is extremely long (273 lines) with high cognitive complexity, handling multiple responsibilities. It should be broken down into smaller, focused functions each handling a specific aspect of the drag operation.
The function's complexity makes it hard to maintain, test, or debug. Here's a suggested approach to refactoring:
- Extract the body element handling into a separate function
- Extract the sibling node handling logic into a separate function
- Extract the container placement logic into a separate function
- Create a main coordinator function that calls these specialized functions
For example:
// Main function coordinates the drag movement const moveMultiDrag = (event: MouseEvent): boolean => { if (!multiDragState.keydown || multiStateLength.value <= 1) return false const { clientX, clientY } = event const currentMousePos: Position = { x: clientX, y: clientY } // Handle drag initialization if (!handleDragInitialization(currentMousePos)) { return true; } // Update mouse position multiDragState.mouse = currentMousePos // Initialize drag state if needed if (!multiDragState.draging && multiDragState.dragStarted) { multiDragState.draging = true } // If not dragging yet, exit early if (!multiDragState.draging) { return true } const targetElement = getElement(event.target as HTMLElement) // Handle special case: body or no element if (!targetElement) { return handleBodyOrNoElement(event); } // Handle regular elements return handleRegularElement(event, targetElement); } // Helper functions would handle the specific casesThis would make the code more maintainable and easier to understand.
738-780: 🛠️ Refactor suggestionFix ordering issues when dragging multiple elements to containers.
The function attempts to maintain the original order of elements but has an issue with the sorting logic. Specifically, when dragging to the bottom of a container, it needs to ensure that elements maintain their vertical order.
// 处理页面底部和容器的情况 if (isBodyTarget && position === POSITION.BOTTOM) { // 放置到页面底部，始终保持从上到下的顺序 const reorderedOperations = [...sortedOperations].reverse() reorderedOperations.forEach((op) => { insertNodeToTarget(op, isBodyTarget, targetId) }) } else if (position === POSITION.IN) { // 放置到容器内部，应该保持原始从上到下的顺序 - sortedOperations.forEach((op) => { + // 确保元素在容器内从上到下排列，不管容器的方向 + const containerOps = position === POSITION.BOTTOM ? [...sortedOperations].reverse() : sortedOperations; + containerOps.forEach((op) => { insertNodeToTarget(op, isBodyTarget, targetId) }) } else { // 其他情况按照排序后的顺序插入 sortedOperations.forEach((op) => { insertNodeToTarget(op, isBodyTarget, targetId) }) }
🧹 Nitpick comments (10)
packages/canvas/container/src/composables/useMultiDrag.ts (10)
89-90: Consider using a constant from a shared configuration file.The
DRAG_THRESHOLDvalue should ideally be managed in a centralized configuration file rather than defined locally, especially if this threshold might be used in other drag-related functionality.
98-130: ThestartMultiDragfunction should capture selected nodes only once.The function redundantly processes selected nodes twice - once when creating
multiDragState.nodesand again when calculating offsets. This causes unnecessary iteration and DOM access.const startMultiDrag = (event: MouseEvent, element: HTMLElement): boolean => { if (multiStateLength.value <= 1) return false // 检查点击的元素是否是已选中的节点之一 const clickedNodeId = element?.getAttribute(NODE_UID) if (!clickedNodeId || !(multiSelectedStates.value as SelectState[]).some((state) => state.id === clickedNodeId)) { return false } const { clientX, clientY } = event multiDragState.keydown = true multiDragState.dragStarted = false multiDragState.draging = false multiDragState.initialMousePos = { x: clientX, y: clientY } multiDragState.targetNodeId = clickedNodeId - multiDragState.nodes = toRaw(multiSelectedStates.value as SelectState[]).map((state) => state.schema) + + // Process selected states once and store both nodes and offsets + const selectedStates = toRaw(multiSelectedStates.value as SelectState[]); + multiDragState.nodes = selectedStates.map((state) => state.schema) // 计算每个节点相对于鼠标的偏移量 - ;(multiSelectedStates.value as SelectState[]).forEach((state) => { + selectedStates.forEach((state) => { const elem = querySelectById(state.id) if (elem) { const { x, y } = elem.getBoundingClientRect() multiDragState.offsets.set(state.id, { offsetX: clientX - x, offsetY: clientY - y, initialX: x, initialY: y }) } }) return true }
161-167: Use built-in Vector calculation utilities if available.The distance calculation function is a basic utility that might already exist elsewhere in the codebase. Consider checking if a similar function exists in a utility module to maintain DRY principles.
305-318: Consider caching the body rectangle.The body's dimensions don't change frequently during drag operations, so calculating
bodyRectmultiple times is inefficient. Consider caching this value or extracting it to a separate utility function.
552-568: Improve the structure and return type ofgetTargetNodeInfo.The function returns an object with mixed naming conventions - some properties have "target" prefix while others have "final" prefix. Additionally, the return type is not explicitly defined, which could lead to type errors.
+ interface TargetNodeInfo { + targetNode: NodeSchema | undefined; + targetParent: NodeSchema | undefined; + isBodyTarget: boolean; + finalTargetNode: NodeSchema | undefined; + finalTargetParent: NodeSchema | null; + } - const getTargetNodeInfo = (targetId: string) => { + const getTargetNodeInfo = (targetId: string): TargetNodeInfo => { const { getNodeWithParentById, getSchema } = useCanvas() const { node: targetNode, parent: targetParent } = getNodeWithParentById(targetId) || {} const isBodyTarget = targetId === 'body' // 如果目标是body，使用页面schema作为目标节点 const finalTargetNode = isBodyTarget ? getSchema() : targetNode const finalTargetParent = isBodyTarget ? null : targetParent return { targetNode, targetParent, isBodyTarget, finalTargetNode, finalTargetParent } }
644-657: Fix potential undefined access in sort comparison.The sort comparison function doesn't handle cases where
posAorposBis undefined properly. Although there's a check that returns 0 if either is undefined, the TypeScript compiler might still flag potential undefined access.const sortedOperations = [...operations].sort((a, b) => { const posA = positions.get(a.sourceId) const posB = positions.get(b.sourceId) if (!posA || !posB) return 0 - // 先按垂直位置排序 - if (Math.abs(posA.top - posB.top) > 5) { - return posA.top - posB.top - } - - // 如果垂直位置接近，则按水平位置排序 - return posA.left - posB.left + // With type safety ensured + // 先按垂直位置排序 + if (Math.abs(posA.top - posB.top) > 5) { + return posA.top - posB.top + } + + // 如果垂直位置接近，则按水平位置排序 + return posA.left - posB.left })
783-795: Use more descriptive variable names for timeouts.The timeout duration (150ms) is a magic number. Use a named constant with a descriptive name to explain why this specific delay was chosen.
// 清理拖拽状态 const cleanupDragState = () => { + // Delay to ensure DOM updates are complete before cleaning up state + const DRAG_CLEANUP_DELAY_MS = 150; // 清理拖拽状态，但保留多选状态 setTimeout(() => { // 只清理拖拽相关状态，不清理多选状态 Object.assign(multiDragState, { ...initialMultiDragState, nodes: [] // 确保清空nodes数组，避免影响后续操作 }) // 清除单选拖动状态 Object.assign(dragState, initialDragState) - }, 150) + }, DRAG_CLEANUP_DELAY_MS) }
834-836: Use direct reactive values for better performance.The
isMultiDraggingcomputed property could be more performant by directly accessing the reactive values without creating unnecessary dependencies.// 判断是否处于多选拖拽状态 const isMultiDragging = computed(() => { - return multiDragState.draging && multiDragState.dragStarted && multiStateLength.value > 1 + // Directly check the current value of multiSelectedStates to avoid unnecessary reactivity + return multiDragState.draging && + multiDragState.dragStarted && + (multiSelectedStates.value as SelectState[]).length > 1 })
839-873: Optimize the computed property with early return pattern.The
getMultiDragPositionTextcomputed property should use early returns for clarity and to avoid unnecessary condition evaluation.// 获取多选拖拽的位置描述 const getMultiDragPositionText: ComputedRef<string> = computed(() => { if (!isMultiDragging.value) return '' const { position, forbidden, id } = lineState // 获取目标节点的组件名称，用于更详细的提示 let targetComponentName = '' if (id && id !== 'body') { const targetElement = querySelectById(id) if (targetElement) { targetComponentName = targetElement.getAttribute(NODE_TAG) || '' } } else if (id === 'body') { targetComponentName = '页面' } if (forbidden) { return `当前位置不允许放置 (${targetComponentName || '目标节点'})` } + // Early return for empty position + if (!position) return '' switch (position) { case POSITION.TOP: return `放置到 ${targetComponentName || '目标节点'} 上方` case POSITION.BOTTOM: return `放置到 ${targetComponentName || '目标节点'} 下方` case POSITION.LEFT: return `放置到 ${targetComponentName || '目标节点'} 左侧` case POSITION.RIGHT: return `放置到 ${targetComponentName || '目标节点'} 右侧` case POSITION.IN: return `放置到 ${targetComponentName || '容器'} 内部` default: return '' } })
1-883: Overall: Refactor this large composable into smaller, focused modules.This composable is extremely long (883 lines) with high complexity. It should be refactored into multiple smaller modules, each handling a specific aspect of the multi-drag functionality:
- A core state manager
- A drag initialization and position calculator
- A drop target validator
- A drag operation executor
- UI feedback handlers
This would significantly improve maintainability, testability, and readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/canvas/container/src/composables/useMultiDrag.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/canvas/container/src/composables/useMultiDrag.ts (3)
packages/canvas/container/src/container.ts (16)
PositionType(50-50)querySelectById(374-384)POSITION(41-48)lineState(137-139)allowInsert(490-509)isAncestor(511-526)getConfigure(471-482)dragState(99-101)initialDragState(52-61)getElement(231-253)getDocument(80-80)getSchema(96-96)insertNode(849-883)syncNodeScroll(685-688)removeNode(341-346)getController(78-78)packages/canvas/container/src/composables/useMultiSelect.ts (1)
useMultiSelect(20-82)packages/register/src/hooks.ts (1)
useCanvas(77-77)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/canvas/container/src/composables/useMultiDrag.ts (1)
132-159: 🛠️ Refactor suggestionEnsure position calculations handle edge cases consistently.
Current position calculation assumes that
rectandconfigureparameters are always valid, but there's no error handling or fallback for invalid inputs. This could lead to unexpected behavior if the parameters are null or undefined.const calculateDropPosition = ( event: MouseEvent, rect: DOMRect, configure: { isContainer?: boolean } | null ): PositionType => { + // Validate inputs + if (!rect) { + return POSITION.BOTTOM; // Default fallback position + } + const { clientX: mouseX, clientY: mouseY } = event // 参考单选节点的实现，使用更精确的计算方式 const yAbs = Math.min(20, rect.height / 3) const xAbs = Math.min(20, rect.width / 3) // 优先判断是否在边缘区域 if (mouseY < rect.top + yAbs) { return POSITION.TOP } else if (mouseY > rect.bottom - yAbs) { return POSITION.BOTTOM } else if (mouseX < rect.left + xAbs) { return POSITION.LEFT } else if (mouseX > rect.right - xAbs) { return POSITION.RIGHT } else if (configure?.isContainer) { // 如果是容器，且鼠标在中间区域，则放置到容器内 return POSITION.IN } // 默认放置到底部 return POSITION.BOTTOM }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/canvas/container/src/CanvasContainer.vue (1)
139-150: Consider making isMultiDragging a computed property.The
isMultiDraggingvariable is destructured directly from the composable. Consider converting it to a computed property to prevent unnecessary re-evaluations on each render.
🧹 Nitpick comments (2)
packages/canvas/container/src/CanvasContainer.vue (2)
264-264: Remove unused variable and eslint comment.The
isScrollingvariable is declared and set in the scroll event listener, but never used in conditional logic.- // eslint-disable-next-line @typescript-eslint/no-unused-vars - let isScrolling = falseAnd remove the corresponding code in the scroll event listener:
win.addEventListener('scroll', () => { - isScrolling = true })
325-356: Extract node selection logic to a separate function.The mouseup event handler contains complex logic for handling clicks on multi-selected nodes. Consider extracting this to a named function for improved readability.
+ // Handle clicks on multi-selected nodes without dragging + const handleMultiSelectNodeClick = (ev) => { + if (multiDragState.keydown && !multiDragState.dragStarted && multiStateLength.value > 1) { + const element = getElement(ev.target) + if (element) { + const clickedNodeId = element?.getAttribute(NODE_UID) + // Only switch to single selection when clicking on a node in the multi-selection + if (clickedNodeId && multiSelectedStates.value.some((state) => state.id === clickedNodeId)) { + selectNode(clickedNodeId) + } + } + } + } win.addEventListener('mouseup', (ev) => { handleCanvasEvent(() => { if (ev.button === 0 && isMouseDown.value) { isMouseDown.value = false // Determine if we need to switch to single selection - // Only needed when clicking a multi-selected node without dragging - if (multiDragState.keydown && !multiDragState.dragStarted && multiStateLength.value > 1) { - const element = getElement(ev.target) - if (element) { - const clickedNodeId = element?.getAttribute(NODE_UID) - // Only switch to single selection when clicking on a node in the multi-selection - if (clickedNodeId && multiSelectedStates.value.some((state) => state.id === clickedNodeId)) { - selectNode(clickedNodeId) - } - } - } + handleMultiSelectNodeClick(ev) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)packages/canvas/container/src/composables/useMultiDrag.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/canvas/container/src/composables/useMultiDrag.ts
🔇 Additional comments (11)
packages/canvas/container/src/CanvasContainer.vue (11)
11-11: New prop allows visual state indication for multi-dragging.The added
:isMultiDraggingprop provides the canvas-action component with information about the current drag state, which is essential for proper visual feedback during multi-selection drag operations.
16-22: Multi-drag indicator component adds visual feedback.This new component provides essential visual feedback when dragging multiple nodes, showing position indicators and potentially the count of nodes being dragged.
73-75: Necessary imports for multi-drag functionality.The new
CanvasMultiDragIndicatorcomponent anduseMultiDragcomposable are correctly imported to support the multi-drag feature.
105-106: Component registration is properly updated.Both
CanvasViewerSwitcherand the newCanvasMultiDragIndicatorare correctly registered in the components list.
127-137: Well-structured drag type enum improves code readability.The
DRAG_TYPEenum andcurrentDragTypestate clearly define the possible drag states, making the code more maintainable and easier to understand.
161-165: Clean helper function for resetting drag states.The
clearAllDragStatesfunction effectively centralizes the logic for resetting all drag-related states, which helps prevent state inconsistencies and code duplication.
172-208: Improved event handling prioritization.The updated
setCurrentNodefunction appropriately prioritizes right-click menu handling, then multi-drag operations, and finally falls back to single node operations. The explicit state transitions make the code flow clear.
298-322: Clear switch structure for drag type handling.The mousemove event listener now has a clear structure with a switch statement that handles each drag type appropriately. The logic for determining the drag type when it hasn't been set yet is well organized.
359-372: Consistent drag event handling.The dragover event handler uses the same switch pattern as the mousemove handler, maintaining consistency across the codebase.
375-389: Consistent drop event handling.The drop event handler follows the same pattern as other event handlers, maintaining a consistent approach to drag operation management across different event types.
477-479: Required state variables exposed.All necessary state variables for the multi-drag feature are correctly exposed in the component's return object.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/canvas/container/src/CanvasContainer.vue (1)
139-150: Consider makingisMultiDragginga computed property.As mentioned in a previous review comment, converting
isMultiDraggingto a computed property could optimize performance by preventing unnecessary method calls during re-renders.
🧹 Nitpick comments (3)
packages/canvas/container/src/CanvasContainer.vue (3)
308-327: Simplify drag type determination logic in mousemove.The current implementation has redundant checks to determine the drag type in the mousemove handler when none is set. The drag type should ideally be determined only once during mousedown.
case DRAG_TYPE.NONE: // 如果尚未确定拖拽类型，尝试确定 if (isMouseDown.value) { - if (multiDragState.keydown) { + // If mousedown occurred but drag type wasn't set, we can determine it based on state + if (multiDragState.keydown && multiStateLength.value > 0) { currentDragType.value = DRAG_TYPE.MULTI moveMultiDrag(ev) } else if (dragState.element) { currentDragType.value = DRAG_TYPE.SINGLE dragMove(ev, true) + } else { + // No valid drag conditions met, clear mouse state + isMouseDown.value = false } } break
339-348: Extract node selection logic to the composable.This node selection logic (determining when to switch from multi-select to single-select) could be moved to the
useMultiSelectcomposable for better separation of concerns.
351-359: Consider refactoring repetitive switch statements.There are several nearly identical switch statements handling drag types in different event handlers. Consider extracting this logic into a helper function to reduce duplication.
+ // Add this helper function + const handleDragByType = (eventType, event) => { + switch (currentDragType.value) { + case DRAG_TYPE.MULTI: + if (eventType === 'move') return moveMultiDrag(event); + if (eventType === 'end') return endMultiDrag(); + break; + case DRAG_TYPE.SINGLE: + if (eventType === 'move') return dragMove(event, eventType === 'mousemove'); + if (eventType === 'end') return onMouseUp(event); + break; + } + } // Then replace switch statements with calls to this function: // e.g., handleDragByType('move', ev) for mousemove/dragover // e.g., handleDragByType('end', ev) for mouseup/dropAlso applies to: 371-378, 386-393
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(3 hunks)packages/canvas/container/src/container.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/canvas/container/src/composables/useMultiSelect.ts
- packages/canvas/container/src/container.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (8)
packages/canvas/container/src/CanvasContainer.vue (8)
11-11: Good usage ofisMultiDraggingprop to indicate multi-drag state.This prop allows
CanvasActioncomponent to visually represent the multi-dragging state, which enhances the user experience by providing visual feedback during the drag operation.
16-22: Excellent addition of the multi-drag indicator component.The new
CanvasMultiDragIndicatorcomponent with all necessary props provides visual feedback during multi-drag operations, which is essential for a good user experience. This component handles showing drag counts, positions, and drag state visualizations.
73-75: The imports for multi-drag functionality are well organized.Proper imports for the new components and composables are included, which separates concerns and follows good architectural patterns.
127-138: Well-structured enum and reactive state for drag types.The
DRAG_TYPEenum clearly defines the possible drag states with descriptive comments, andcurrentDragTypereactive state tracks the current drag operation type. This addresses previous feedback about having a drag state variable to determine execution paths.
161-165: Good abstraction withclearAllDragStates.Creating a centralized function to clear all drag states improves code maintainability and reduces the chance of inconsistent state cleanup across different event handlers.
184-188: Clear priority handling for multi-drag operations.The multi-drag start attempt is correctly prioritized over single-node selection, which creates a consistent and predictable interaction model.
462-487: Well-exposed state variables for component use.All necessary drag state variables and helper functions are properly exposed from the setup function, making them available to child components and template.
1-24: Good component integration in the template.The template structure properly incorporates the new multi-drag indicator component and passes the necessary props to both that and the
CanvasActioncomponent, ensuring proper visual feedback during multi-drag operations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/canvas/container/src/CanvasContainer.vue (1)
139-150: Consider refactoring isMultiDragging to a computed property.The
isMultiDraggingflag is being imported directly from theuseMultiDragcomposable, but it could be more efficient to define it as a computed property based on the current drag type.-const { - startMultiDrag, - moveMultiDrag, - endMultiDrag, - isMultiDragging, - getMultiDragPositionText, - multiDragState, - cleanupDragState -} = useMultiDrag() +const { + startMultiDrag, + moveMultiDrag, + endMultiDrag, + getMultiDragPositionText, + multiDragState, + cleanupDragState +} = useMultiDrag() +const isMultiDragging = computed(() => currentDragType.value === DRAG_TYPE.MULTI && multiDragState.dragStarted)
🧹 Nitpick comments (3)
packages/canvas/container/src/CanvasContainer.vue (3)
304-329: Refactor mousemove event handler for better readability.The switch-case structure is appropriate, but the nested condition in the NONE case makes the code harder to follow. Consider extracting this logic to a separate function.
win.addEventListener('mousemove', (ev) => { handleCanvasEvent(() => { // 根据当前拖拽类型执行相应操作 switch (currentDragType.value) { case DRAG_TYPE.MULTI: moveMultiDrag(ev) break case DRAG_TYPE.SINGLE: dragMove(ev, true) break case DRAG_TYPE.NONE: - // 如果尚未确定拖拽类型，尝试确定 - if (isMouseDown.value) { - if (multiDragState.keydown) { - currentDragType.value = DRAG_TYPE.MULTI - moveMultiDrag(ev) - } else if (dragState.element) { - currentDragType.value = DRAG_TYPE.SINGLE - dragMove(ev, true) - } - } + determineDragType(ev) break } }) }) // 新增函数，用于确定拖拽类型 const determineDragType = (ev) => { if (!isMouseDown.value) return if (multiDragState.keydown) { currentDragType.value = DRAG_TYPE.MULTI moveMultiDrag(ev) } else if (dragState.element) { currentDragType.value = DRAG_TYPE.SINGLE dragMove(ev, true) } }
331-362: Add debounce to mouseup event handler.The mouseup event handler contains complex logic for handling different scenarios. Since these events can fire rapidly in succession, consider adding debounce to prevent potential performance issues.
+import { debounce } from 'lodash-es' // 其他代码... +const debouncedMouseUpHandler = debounce((ev) => { + if (ev.button === 0 && isMouseDown.value) { + isMouseDown.value = false + + // 判断是否需要切换到单选状态 + // 只有当点击多选节点但没有拖动时，才需要切换到单选状态 + if (multiDragState.keydown && !multiDragState.dragStarted && multiStateLength.value > 1) { + const element = getElement(ev.target) + if (element) { + const clickedNodeId = element?.getAttribute(NODE_UID) + // 只有点击的是多选节点中的一个时才切换到单选 + if (clickedNodeId && multiSelectedStates.value.some((state) => state.id === clickedNodeId)) { + selectNode(clickedNodeId) + } + } + } + } + + // 根据当前拖拽类型执行相应的结束操作 + switch (currentDragType.value) { + case DRAG_TYPE.MULTI: + endMultiDrag() + break + case DRAG_TYPE.SINGLE: + onMouseUp(ev) + break + } + + clearAllDragStates() +}, 50) win.addEventListener('mouseup', (ev) => { handleCanvasEvent(() => { - if (ev.button === 0 && isMouseDown.value) { - isMouseDown.value = false - - // 判断是否需要切换到单选状态 - // 只有当点击多选节点但没有拖动时，才需要切换到单选状态 - if (multiDragState.keydown && !multiDragState.dragStarted && multiStateLength.value > 1) { - const element = getElement(ev.target) - if (element) { - const clickedNodeId = element?.getAttribute(NODE_UID) - // 只有点击的是多选节点中的一个时才切换到单选 - if (clickedNodeId && multiSelectedStates.value.some((state) => state.id === clickedNodeId)) { - selectNode(clickedNodeId) - } - } - } - } - - // 根据当前拖拽类型执行相应的结束操作 - switch (currentDragType.value) { - case DRAG_TYPE.MULTI: - endMultiDrag() - break - case DRAG_TYPE.SINGLE: - onMouseUp(ev) - break - } - - clearAllDragStates() + debouncedMouseUpHandler(ev) }) })
456-480: Add typing information to the exposed state and functions.While the implementation exposes all the necessary state and helper functions, adding TypeScript type annotations would improve code documentation and IDE support.
return { - isMouseDown, - iframe, - dragState, - hoverState, - inactiveHoverState, - computedSelectState, - lineState, - multiSelectedStates, - multiStateLength, - removeNodeById, - selectSlot, - canvasState, - insertComponent, - insertPanel, - containerPanel, - settingModel, - target, - showSettingModel, - insertPosition, - insertContainer, - loading, - srcAttrName, - isMultiDragging, - multiDragState, - getMultiDragPositionText + isMouseDown, // Ref<boolean> + iframe, // Ref<HTMLIFrameElement | null> + dragState, // State object for single drag operations + hoverState, // State object for hover operations + inactiveHoverState, // State object for inactive hover operations + computedSelectState, // ComputedRef for selected state + lineState, // State object for line indicators + multiSelectedStates, // Ref<Array> of selected states + multiStateLength, // ComputedRef<number> for number of selected states + removeNodeById, // Function to remove a node + selectSlot, // Function to select a slot + canvasState, // State object for canvas + insertComponent, // Function to insert a component + insertPanel, // Ref for insert panel + containerPanel, // Ref for container panel + settingModel, // Function to open settings + target, // Ref for event target + showSettingModel, // Ref<boolean> for settings visibility + insertPosition, // Ref for insert position + insertContainer, // Ref for insert container + loading, // ComputedRef<boolean> for loading state + srcAttrName, // ComputedRef<string> for src attribute name + isMultiDragging, // Ref<boolean> indicating multi-drag state + multiDragState, // State object for multi-drag operations + getMultiDragPositionText // Function to get position text for multi-drag }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/canvas/container/src/CanvasContainer.vue(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (6)
packages/canvas/container/src/CanvasContainer.vue (6)
11-11: Good addition of visual indicators for multi-drag operations.The new
isMultiDraggingprop on CanvasAction and the CanvasMultiDragIndicator component provide valuable visual feedback to users during multi-drag operations. This enhancement improves the user experience by making it clear when multiple nodes are being dragged.Also applies to: 16-22
127-138: Good use of an enum for drag states.Creating a
DRAG_TYPEenum with clearly named states makes the code more readable and maintainable. The approach of tracking the current drag type with a reactive variable is a clean way to manage the application state.
160-165: Good practice with the clearAllDragStates function.Centralizing the cleanup logic in a single function helps maintain consistency and prevents state leaks. This is a good approach for ensuring all drag-related states are properly reset after operations complete.
183-189: Improved event handling logic for multi-drag operations.The code now correctly prioritizes multi-drag operations when appropriate, improving the user experience. The early return pattern when multi-drag starts prevents conflicting states from occurring.
370-375: Good implementation of type-specific dragover handling.The code now correctly routes dragover events to the appropriate handler based on the current drag type. This ensures consistent behavior between mouse and drag events.
382-389: Good implementation of type-specific drop handling.Similar to the dragover handling, the drop event is properly routed to the appropriate handler. The addition of clearAllDragStates ensures that drag states are always cleaned up, which prevents state inconsistencies.
3 participants
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?
Issue Number: N/A
What is the new behavior?
操作步骤：
1.1 点击 Ctrl + 鼠标左键进行节点多选；
1.2 节点多选结束后，使用鼠标左键点击多选节点进行拖拽；
1.3 拖拽到指定位置、松开鼠标左键、拖拽结束；
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style