fix: insert logic and multi node insert issue #1150
Conversation
WalkthroughThe pull request streamlines node insertion and clipboard handling in the canvas modules. In the DesignCanvas API, the Changes
Sequence Diagram(s)Clipboard Paste FlowsequenceDiagram
participant U as User
participant KE as Keyboard Handler
participant CP as Clipboard Processor
participant CI as Canvas Inserter
U->>KE: Trigger Paste Event (e.g., Ctrl+V)
KE->>CP: Extract node list from clipboard
CP->>CP: Validate nodes & iterate over list
CP->>CI: Insert each node via operationTypeMap
CI-->>CP: Confirm insertion
CP-->>KE: Return paste result
Node Insertion FlowsequenceDiagram
participant Caller as operationTypeMap
participant Parent as ParentNode
Caller->>Parent: Determine insertion index based on position ('before' or default)
Parent-->>Caller: Provide current children count
Caller->>Parent: Insert new node at calculated index using splice
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
packages/canvas/DesignCanvas/src/api/useCanvas.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/canvas".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/canvas/.eslintrc.cjs » @vue/eslint-config-typescript". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/canvas/container/src/utils.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/canvas".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/canvas/.eslintrc.cjs » @vue/eslint-config-typescript". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/canvas/container/src/keyboard.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/canvas".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/canvas/.eslintrc.cjs » @vue/eslint-config-typescript". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (2)
packages/canvas/container/src/keyboard.js (1)
105-115: Enhanced clipboard paste to support multiple nodes.The function now handles a list of nodes instead of a single node, addressing the multi-node insertion issue mentioned in the PR objectives. The logic correctly iterates through each node in the list and applies appropriate insertion logic based on component compatibility.
Consider adding error handling in case any individual node insertion fails, to prevent partial paste operations:
const handleClipboardPaste = (nodeList, schema, parent) => { if (!nodeList.length) return + try { nodeList.forEach((node) => { if (node?.componentName && schema?.componentName && allowInsert(getConfigure(schema.componentName), node)) { insertNode({ parent, node: schema, data: node }, POSITION.IN) } else { insertNode({ parent, node: schema, data: node }, POSITION.BOTTOM) } }) + } catch (error) { + console.error('Error pasting nodes:', error) + } }packages/canvas/container/src/utils.js (1)
32-63: Enhanced schema translation with better validation and fallbacks.The improved function now:
- Properly validates input
- Handles both array and single object formats
- Provides a sensible fallback for invalid data
This makes the clipboard functionality more robust when handling different data formats.
Consider adding optional chaining for better error prevention as suggested by the static analysis:
- if (Array.isArray(parsedData) && parsedData.every((item) => item && item.componentName)) { + if (Array.isArray(parsedData) && parsedData.every((item) => item?.componentName)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/canvas/DesignCanvas/src/api/useCanvas.js(2 hunks)packages/canvas/container/src/container.js(0 hunks)packages/canvas/container/src/keyboard.js(2 hunks)packages/canvas/container/src/utils.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/canvas/container/src/container.js
🧰 Additional context used
🪛 Biome (1.9.4)
packages/canvas/container/src/utils.js
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (6)
packages/canvas/DesignCanvas/src/api/useCanvas.js (2)
325-326: Simplified insertion logic for 'before' position.The code now handles the case where the reference node isn't found (index === -1) by defaulting to the start of the array, which is more robust than the previous implementation.
338-339: Improved default insertion logic.The simplified logic now handles the case where the reference node isn't found by appending to the end of the array, and otherwise inserts after the reference node. This is more intuitive and reduces the chance of insertion errors.
packages/canvas/container/src/keyboard.js (3)
117-122: Enhanced copy operation to support multiple nodes.The copy operation now properly handles multiple selected nodes, storing them as an array in the clipboard. This implementation works well with the updated paste functionality.
126-127: Updated clipboard retrieval to fetch node list.The code now expects to retrieve a list of nodes from the clipboard rather than a single node, properly aligning with the changes in the utils.js file.
133-133: Updated paste handler to use new multi-node functionality.The clipboard paste event now correctly calls the enhanced paste function with the node list.
packages/canvas/container/src/utils.js (1)
19-29: Improved clipboard data handling for node lists.The function now accepts a string representation of nodes rather than a node object, which is more flexible and supports the multi-node clipboard functionality. The validation is also improved to ensure valid string data.
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?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor