Move Frame Vue Nodes #5886
Move Frame Vue Nodes #5886
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/02/2025, 12:16:38 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 10/02/2025, 12:26:50 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
dosubot
bot
added
the
size:M
| const bounds = item.getBounding() | ||
| allBounds.push([bounds[0], bounds[1], bounds[2], bounds[3]] as const) | ||
| } else if (item instanceof LGraphGroup) { | ||
| const bounds = item.boundingRect |
There was a problem hiding this comment.
If Node and Group both exposed the same method to get the bounds, you could eliminate the instanceof check, right?
| for (const item of allItems) { | ||
| item.move(deltaX, deltaY, true) | ||
|
|
||
| if (LiteGraph.vueNodesMode) { |
There was a problem hiding this comment.
This is a pretty big conditional, could you break it up or flip it to a guard clause?
| const nodesInMovingGroups = new Set<LGraphNode>() | ||
| const nodesToMove: Array<{ | ||
| node: LGraphNode | ||
| newPos: { x: number; y: number } | ||
| }> = [] |
There was a problem hiding this comment.
For clarity, could you either extract a method with a name or use functional chaining here?
These seem like operations that could be useful otherwise later, but I'm more concerned with readability.
| const isNode = item instanceof LGraphNode | ||
| if (isNode) { | ||
| const node = item as LGraphNode | ||
| if (nodesInMovingGroups.has(node)) { | ||
| continue | ||
| } | ||
| nodesToMove.push({ | ||
| node, | ||
| newPos: { | ||
| x: node.pos[0] + deltaX, | ||
| y: node.pos[1] + deltaY | ||
| } | ||
| }) |
There was a problem hiding this comment.
The checks here with the very similar contents make me think there's a worthwhile abstraction to be had in LGraphNode and LGraphGroup.
This pull request improves the selection and movement logic for groups and nodes on the LiteGraph canvas, especially when using Vue-based node rendering. The most notable changes are the addition of proper bounding box handling for groups and a new coordinated movement mechanism that updates both LiteGraph internals and the Vue layout store when dragging nodes and groups.
Selection and bounding box calculation:
LGraphGroupbounding rectangles when calculating the selection toolbox position, so groups are now properly considered in selection overlays. [1] [2]Node and group movement synchronization (Vue nodes mode):
LGraphCanvasfor Vue nodes mode: when dragging, groups and their child nodes are moved together, and all affected node positions are batch-updated in both LiteGraph and the Vue layout store viamoveNode. This ensures canvas and UI stay in sync.These changes make group selection and movement more robust and ensure that UI and internal state remain consistent when using the Vue-based node system.
Screen.Recording.2025-10-02.at.01.13.02.mov
┆Issue is synchronized with this Notion page by Unito