-
Notifications
You must be signed in to change notification settings - Fork 8
Add node containment support to composite tasks react flow components (do, for, fork, try) #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
489e3aa
a6c7064
1c721ca
ed4cea1
84ee567
4e56044
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@serverlessworkflow/diagram-editor": minor | ||
| --- | ||
|
|
||
| Add containment support integrated with auto-layout. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,39 +39,41 @@ export type Size = { | |
| export type WayPoints = Point[]; | ||
|
|
||
| export const ROOT_LAYOUT_OPTIONS: LayoutOptions = { | ||
| "elk.algorithm": "org.eclipse.elk.layered", | ||
| "elk.hierarchyHandling": "INCLUDE_CHILDREN", | ||
| "elk.direction": "DOWN", | ||
| "org.eclipse.elk.layered.layering.strategy": "INTERACTIVE", | ||
| "org.eclipse.elk.edgeRouting": "ORTHOGONAL", | ||
| "elk.layered.unnecessaryBendpoints": "true", | ||
| "org.eclipse.elk.algorithm": "org.eclipse.elk.layered", | ||
| "org.eclipse.elk.direction": "DOWN", | ||
| "org.eclipse.elk.layered.nodePlacement.strategy": "BRANDES_KOEPF", | ||
| "org.eclipse.elk.layered.nodePlacement.bk.fixedAlignment": "BALANCED", | ||
| "org.eclipse.elk.layered.nodePlacement.bk.edgeStraightening": "IMPROVE_STRAIGHTNESS", | ||
| "org.eclipse.elk.layered.cycleBreaking.strategy": "DEPTH_FIRST", | ||
| "org.eclipse.elk.insideSelfLoops.activate": "true", | ||
| "elk.separateConnectedComponents": "false", | ||
| "org.eclipse.elk.layered.nodePlacement.bk.edgeStraightening": "true", | ||
| "org.eclipse.elk.layered.nodePlacement.favorStraightEdges": "true", | ||
| "org.eclipse.elk.layered.considerModelOrder.strategy": "EDGES", | ||
| "org.eclipse.elk.layered.priority.straightness": "10", | ||
| "org.eclipse.elk.hierarchyHandling": "INCLUDE_CHILDREN", | ||
| "org.eclipse.elk.layered.crossingMinimization.strategy": "LAYER_SWEEP", | ||
| "org.eclipse.elk.edgeRouting": "ORTHOGONAL", | ||
| "org.eclipse.elk.layered.unnecessaryBendpoints": "true", | ||
| "org.eclipse.elk.layered.cycleBreaking.strategy": "GREEDY_MODEL_ORDER", | ||
| "org.eclipse.elk.layered.considerModelOrder.crossingCounterNodeInfluence": "0.001", | ||
| "elk.layered.crossingMinimization.strategy": "INTERACTIVE", | ||
| spacing: "75", | ||
| "spacing.componentComponent": "70", | ||
| "spacing.nodeNodeBetweenLayers": "80", | ||
| "elk.layered.spacing.edgeNodeBetweenLayers": "40", | ||
| "org.eclipse.elk.spacing.edgeNode": "24", | ||
| "org.eclipse.elk.layered.spacing.edgeNode": "24", | ||
| "org.eclipse.elk.layered.spacing.edgeNodeBetweenLayers": "40", | ||
| "org.eclipse.elk.layered.spacing.nodeNode": "24", | ||
| "org.eclipse.elk.layered.spacing.nodeNodeBetweenLayers": "80", | ||
| "org.eclipse.elk.layered.spacing.componentComponent": "70", | ||
| "org.eclipse.elk.layered.mergeEdges": "true", | ||
| }; | ||
|
|
||
| export const PARENT_LAYOUT_OPTIONS: LayoutOptions = { | ||
| ...ROOT_LAYOUT_OPTIONS, | ||
| "elk.padding": "[top=60,left=20,bottom=20,right=20]", | ||
|
handreyrc marked this conversation as resolved.
Outdated
|
||
| }; | ||
|
|
||
| export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): ElkNode { | ||
| // Create a map for easy lookup | ||
| const nodeMap = new Map( | ||
| // Create a map for easy lookup (without width/height initially) | ||
| const nodeMap = new Map<string, ElkNode>( | ||
| reactFlowGraph.nodes.map((node) => [ | ||
| node.id, | ||
| { | ||
| id: node.id, | ||
| width: node.measured?.width ?? DEFAULT_NODE_SIZE.width, | ||
| height: node.measured?.height ?? DEFAULT_NODE_SIZE.height, | ||
| children: [] as ElkNode[], | ||
| edges: [] as ElkExtendedEdge[], | ||
| }, | ||
| ]), | ||
| ); | ||
|
|
@@ -81,24 +83,96 @@ export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): | |
| reactFlowGraph.nodes.forEach((node) => { | ||
| const elkNode = nodeMap.get(node.id)!; | ||
| if (node.parentId && nodeMap.has(node.parentId)) { | ||
| nodeMap.get(node.parentId)!.children.push(elkNode); | ||
| const parentNode = nodeMap.get(node.parentId)!; | ||
| if (!parentNode.children) { | ||
| parentNode.children = []; | ||
| } | ||
| parentNode.children.push(elkNode); | ||
| } else { | ||
| rootChildren.push(elkNode); | ||
| } | ||
| }); | ||
|
|
||
| // edges | ||
| const elkEdges: ElkExtendedEdge[] = reactFlowGraph.edges.map((edge) => ({ | ||
| id: edge.id, | ||
| sources: [edge.source], | ||
| targets: [edge.target], | ||
| })); | ||
| // Apply layout options and dimensions based on whether node has children | ||
| reactFlowGraph.nodes.forEach((node) => { | ||
| const elkNode = nodeMap.get(node.id)!; | ||
| if (elkNode.children && elkNode.children.length > 0) { | ||
| // Nodes with children get layout options but no fixed dimensions | ||
| elkNode.layoutOptions = PARENT_LAYOUT_OPTIONS; | ||
|
handreyrc marked this conversation as resolved.
Outdated
|
||
| } else { | ||
| // Leaf nodes get fixed dimensions | ||
| elkNode.width = node.measured?.width ?? DEFAULT_NODE_SIZE.width; | ||
| elkNode.height = node.measured?.height ?? DEFAULT_NODE_SIZE.height; | ||
| } | ||
| }); | ||
|
|
||
| // Helper function to find the common ancestor of two nodes | ||
| const findCommonAncestor = (sourceId: string, targetId: string): string => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like an helper function which can be declared at top level in this file, receiving
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
| // Build path from source to root | ||
| const sourcePath: string[] = []; | ||
| let currentId: string | undefined = sourceId; | ||
| while (currentId) { | ||
| sourcePath.push(currentId); | ||
| const node = reactFlowGraph.nodes.find((n) => n.id === currentId); | ||
| currentId = node?.parentId; | ||
| } | ||
|
|
||
| // Traverse from target to root and find first common node | ||
| currentId = targetId; | ||
| while (currentId) { | ||
| if (sourcePath.includes(currentId)) { | ||
| return currentId; | ||
| } | ||
| const node = reactFlowGraph.nodes.find((n) => n.id === currentId); | ||
|
handreyrc marked this conversation as resolved.
Outdated
|
||
| currentId = node?.parentId; | ||
| } | ||
|
|
||
| // If no common ancestor found, return "root" | ||
| return "root"; | ||
| }; | ||
|
|
||
| // Nest edges in the appropriate hierarchy level | ||
| const rootEdges: ElkExtendedEdge[] = []; | ||
| reactFlowGraph.edges.forEach((edge) => { | ||
| const elkEdge: ElkExtendedEdge = { | ||
| id: edge.id, | ||
| sources: [edge.source], | ||
| targets: [edge.target], | ||
| }; | ||
|
|
||
| // Find the lowest common ancestor that contains both source and target | ||
| const commonAncestor = findCommonAncestor(edge.source, edge.target); | ||
|
|
||
| if (commonAncestor === "root") { | ||
| rootEdges.push(elkEdge); | ||
| } else { | ||
| const ancestorNode = nodeMap.get(commonAncestor); | ||
| if (ancestorNode) { | ||
| if (!ancestorNode.edges) { | ||
| ancestorNode.edges = []; | ||
| } | ||
| ancestorNode.edges.push(elkEdge); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Clean up empty edges arrays from nodes that don't need them | ||
| const cleanupEmptyEdges = (node: ElkNode) => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also declare this as top level function
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
| if (node.edges && node.edges.length === 0) { | ||
| delete node.edges; | ||
| } | ||
| if (node.children) { | ||
| node.children.forEach(cleanupEmptyEdges); | ||
| } | ||
| }; | ||
|
|
||
| rootChildren.forEach(cleanupEmptyEdges); | ||
|
|
||
| return { | ||
| id: "root", | ||
| layoutOptions: ROOT_LAYOUT_OPTIONS, | ||
| children: rootChildren, | ||
| edges: elkEdges, | ||
| edges: rootEdges, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -116,14 +190,48 @@ function buildElkNodeMap( | |
| return map; | ||
| } | ||
|
|
||
| // Helper function to recursively collect all edges from ELK graph | ||
| function buildElkEdgeMap( | ||
| elkNode: ElkNode, | ||
| map: Map<string, ElkExtendedEdge> = new Map(), | ||
| ): Map<string, ElkExtendedEdge> { | ||
| if (elkNode.edges) { | ||
| for (const edge of elkNode.edges) { | ||
| map.set(edge.id, edge); | ||
| } | ||
| } | ||
| if (elkNode.children) { | ||
| for (const child of elkNode.children) { | ||
| buildElkEdgeMap(child, map); | ||
| } | ||
| } | ||
| return map; | ||
| } | ||
|
|
||
| // Helper function to check if an edge is inside a parent node | ||
| function isEdgeInsideParent( | ||
| edge: { source: string; target: string }, | ||
| graph: ReactFlowGraph, | ||
| ): boolean { | ||
| const sourceNode = graph.nodes.find((n) => n.id === edge.source); | ||
| const targetNode = graph.nodes.find((n) => n.id === edge.target); | ||
|
handreyrc marked this conversation as resolved.
Outdated
|
||
|
|
||
| // Edge is inside a parent if both source and target have the same parentId | ||
| return !!( | ||
| sourceNode?.parentId && | ||
| targetNode?.parentId && | ||
| sourceNode.parentId === targetNode.parentId | ||
| ); | ||
| } | ||
|
handreyrc marked this conversation as resolved.
|
||
|
|
||
| // set | ||
| export function matchReactFlowGraphWithElkLayoutedGraph( | ||
| graph: ReactFlowGraph, | ||
| layoutedGraph: ElkNode, | ||
| ): ReactFlowGraph { | ||
| // Build flat maps for O(1) lookups | ||
| const elkNodeMap = buildElkNodeMap(layoutedGraph); | ||
| const elkEdgeMap = new Map(layoutedGraph.edges?.map((e) => [e.id, e]) || []); | ||
| const elkEdgeMap = buildElkEdgeMap(layoutedGraph); | ||
|
|
||
| // Map node positions | ||
| const layoutedNodes = graph.nodes.map((node) => { | ||
|
|
@@ -143,15 +251,27 @@ export function matchReactFlowGraphWithElkLayoutedGraph( | |
| const layoutedEdges = graph.edges.map((edge) => { | ||
| const elkEdge = elkEdgeMap.get(edge.id); | ||
| if (elkEdge) { | ||
| // Reconstruct data without old wayPoints to avoid stale routing whenever ELK produced this edge. | ||
| // Reconstruct data without old wayPoints to avoid stale routing | ||
| const { wayPoints: _oldWayPoints, ...restData } = edge.data || {}; | ||
| const bendPoints = elkEdge.sections?.flatMap((section) => section.bendPoints || []) || []; | ||
|
|
||
| // Always create new data object, only add wayPoints if there are bend points | ||
| const newData = { ...restData }; | ||
| if (bendPoints.length > 0) { | ||
| // Check if edge is inside a parent node and apply offset | ||
| const isInsideParent = isEdgeInsideParent(edge, graph); | ||
| if (isInsideParent) { | ||
| // There is an incompatibity with the react flow library, the wayPoints are calculated correctly by ELK | ||
| // but the way react flow rendeer edges inside parent nodes cause path distotions. | ||
|
handreyrc marked this conversation as resolved.
Outdated
|
||
| newData.wayPoints = undefined; | ||
| } else { | ||
| newData.wayPoints = bendPoints; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| ...edge, | ||
| data: { | ||
| ...restData, | ||
| ...(bendPoints.length > 0 && { wayPoints: bendPoints }), | ||
| }, | ||
| data: newData, | ||
| }; | ||
| } | ||
| return edge; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.