Save groups in the graph #455
Conversation
|
|
Failed to generate code suggestions for PR |
| graphNode.annotations[ypos] = node.position.y - parentPosition.y + GROUP_NODE_PADDING; | ||
| graphNode.annotations[width] = reactFlowNodesMap.get(node.id)?.width || 200; | ||
| graphNode.annotations[height] = reactFlowNodesMap.get(node.id)?.height || 100; | ||
| graphNode.annotations['parentId'] = flowNode.id; |
There was a problem hiding this comment.
This should be scoped to indicate its for the UI
| nodes.forEach((node) => { | ||
| const graphNode = graph.getNode(node.id); | ||
| if (graphNode) { | ||
| graphNode.annotations[xpos] = node.position.x - parentPosition.x + GROUP_NODE_PADDING; |
There was a problem hiding this comment.
Would these persist if they are changed afterwards ? We don't seem to ever recompute these on changes
There was a problem hiding this comment.
It's not needed. After group creation you don't need to update this. If you move the node inside the group it will automatically change it's xpos and ypos. If you move the group, xpos and ypos of nodes inside should still be the same
| const selectedNodeIds = selectedNodes | ||
| .reduce((acc, node) => { | ||
| if (node.type !== GROUP) { | ||
| return [...acc, node.id]; |
There was a problem hiding this comment.
You wouldn't need to spread each time, this would create a new array, since its bound to the reduce, just push the new node and return the accumulator
| }); | ||
| }); | ||
|
|
||
| const reactFlowNodesMap = new Map<string, Node>( |
There was a problem hiding this comment.
This would be called each render which will hamper performance
There was a problem hiding this comment.
This is inside the onGroup callback. Will be called only once when you group nodes
| @@ -294,9 +336,11 @@ export const SelectionContextMenu = ({ id, nodes }: INodeContextMenuProps) => { | |||
| duplicateNodes(selectedNodeIds); | |||
| }; | |||
|
|
|||
| const hasGroup = selectedNodes.some((node) => node.type === GROUP); | |||
| @@ -0,0 +1,8 @@ | |||
| import { Node } from '../../programmatic/node.js'; | |||
|
|
|||
| export default class NodeDefinition extends Node { | |||
There was a problem hiding this comment.
I don't think we should add all this. This node does nothing, we've cluttered a lot of the code and we're likely going to get inconsistencies from the annotations not being updated correctly between reactflow. We should rather just use the finalizers for serialization and deserialization and store the group info as annotations until we make a more concrete construct that actually does stuff.
| return [...acc, nodeId]; | ||
| }, [] as string[]); | ||
|
|
||
| console.log('duplicate'); |
This PR include:
Fixes #418
Screen.Recording.2024-07-16.at.18.37.42.mov
Screen.Recording.2024-07-18.at.12.34.02.mov