From 4d725a7212ac0891247e1f407dc073ea180303a4 Mon Sep 17 00:00:00 2001 From: Andre Giron Date: Tue, 28 Feb 2023 11:46:33 -0800 Subject: [PATCH] Eng 2450 fix node layout algo (#1029) * Improves layout algorithm for DAG nodes. --- .../components/workflows/ReactFlowCanvas.tsx | 24 ++++++++++- .../workflows/nodes/BaseNode.styles.tsx | 8 ++-- .../workflows/nodes/CheckOperatorNode.tsx | 3 -- .../workflows/nodes/MetricOperatorNode.tsx | 3 -- .../src/components/workflows/nodes/Node.tsx | 40 +++++++++++-------- .../components/workflows/nodes/NodeStatus.tsx | 37 ----------------- src/ui/common/src/reducers/workflow.ts | 19 +++------ 7 files changed, 55 insertions(+), 79 deletions(-) delete mode 100644 src/ui/common/src/components/workflows/nodes/NodeStatus.tsx diff --git a/src/ui/common/src/components/workflows/ReactFlowCanvas.tsx b/src/ui/common/src/components/workflows/ReactFlowCanvas.tsx index 956bfa2a4..0ea5f02c9 100644 --- a/src/ui/common/src/components/workflows/ReactFlowCanvas.tsx +++ b/src/ui/common/src/components/workflows/ReactFlowCanvas.tsx @@ -27,11 +27,30 @@ const ReactFlowCanvas: React.FC = ({ const { edges, nodes } = dagPositionState.result ?? { edges: [], nodes: [] }; + const canvasEdges = edges.map((edge) => { + return { + id: edge.id, + source: edge.source, + target: edge.target, + type: edge.type, + container: 'root', + }; + }); + + const canvasNodes = nodes.map((node) => { + return { + id: node.id, + type: node.type, + data: node.data, + position: node.position, + }; + }); + return ( = ({ defaultZoom={1} edgeTypes={EdgeTypes} minZoom={0.25} + fitView={true} /> ); }; diff --git a/src/ui/common/src/components/workflows/nodes/BaseNode.styles.tsx b/src/ui/common/src/components/workflows/nodes/BaseNode.styles.tsx index 6a6ba8dee..dc16289ca 100644 --- a/src/ui/common/src/components/workflows/nodes/BaseNode.styles.tsx +++ b/src/ui/common/src/components/workflows/nodes/BaseNode.styles.tsx @@ -9,10 +9,10 @@ const BaseNode = styled(Box)({ borderRadius: '8px', borderStyle: 'solid', borderWidth: '2px', - padding: '10px', - maxWidth: '250px', - minHeight: '140px', - maxHeight: '250px', + width: '310px', + height: '160px', + maxWidth: '310px', + maxHeight: '160px', }); export { BaseNode }; diff --git a/src/ui/common/src/components/workflows/nodes/CheckOperatorNode.tsx b/src/ui/common/src/components/workflows/nodes/CheckOperatorNode.tsx index 5d00c4cb4..a847df869 100644 --- a/src/ui/common/src/components/workflows/nodes/CheckOperatorNode.tsx +++ b/src/ui/common/src/components/workflows/nodes/CheckOperatorNode.tsx @@ -92,9 +92,6 @@ const CheckOperatorNode: React.FC = ({ data, isConnectable }) => { color: textColor, borderColor: borderColor, '&:hover': { backgroundColor: hoverColor }, - minHeight: 'unset', - minWidth: '240px', - padding: '0px', }} > = ({ data, isConnectable }) => { color: textColor, borderColor: borderColor, '&:hover': { backgroundColor: hoverColor }, - minHeight: 'unset', - minWidth: '240px', - padding: '0px', }} > = ({ '&:hover': { backgroundColor: hoverColor }, }} > - {icon && ( - - - - )} - - - {label} - + {icon && ( + + + + )} + + {label} + + = ({ execState }) => { - let statusIcon = null; - const runStatus: string = execState.status.toLowerCase(); - - if (runStatus === ExecutionStatus.Succeeded) { - statusIcon = ; - } else if ( - runStatus === ExecutionStatus.Failed && - execState.failure_type === FailureType.UserNonFatal - ) { - statusIcon = ; - } else if (runStatus === ExecutionStatus.Failed) { - statusIcon = ; - } else if (runStatus === ExecutionStatus.Pending) { - statusIcon = ; - } else if (runStatus === ExecutionStatus.Canceled) { - statusIcon = ; - } - - return ( - - {statusIcon} - - ); -}; - -export default NodeStatus; diff --git a/src/ui/common/src/reducers/workflow.ts b/src/ui/common/src/reducers/workflow.ts index 318e0559c..0c727f92a 100644 --- a/src/ui/common/src/reducers/workflow.ts +++ b/src/ui/common/src/reducers/workflow.ts @@ -484,11 +484,10 @@ export const handleGetSelectDagPosition = createAsyncThunk< const mappedNodes = collapsedPosition.nodes.map((node) => { return { id: node.id, - // Width and height set not only the node width/height, but also how far apart we want to position these nodes. - // So using a value of just the width and the height of the node will result in no spacing in between. - // Elk (eclipse layout kernel) also provides many knobs to tweak this as well. + // width of the node in px. width: 300, - height: 300, + // height of the node in px. + height: 150, }; }); @@ -497,15 +496,9 @@ export const handleGetSelectDagPosition = createAsyncThunk< layoutOptions: { 'elk.algorithm': 'layered', 'elk.direction': 'RIGHT', - 'elk.alignment': 'CENTER', - 'elk.spacing.nodeNode': '80', - 'elk.layered.spacing.nodeNodeBetweenLayers': '80', - // https://www.eclipse.org/elk/reference/options/org-eclipse-elk-layered-nodePlacement-strategy.html - 'nodePlacement.strategy': 'NETWORK_SIMPLEX', - 'org.eclipse.elk.edgeRouting': 'SPLINES', - //https://www.eclipse.org/elk/reference/options/org-eclipse-elk-layered-nodePlacement-strategy.html - 'crossingMinimization.strategy': 'INTERACTIVE', - 'crossingMinimization.forceNodeModelOrder': true, + 'elk.spacing.nodeNode': '300', + 'elk.layered.spacing.nodeNodeBetweenLayers': '300', + 'elk.layered.nodePlacement.strategy': 'INTERACTIVE', }, children: mappedNodes, edges: collapsedPosition.edges,