[3392] Prevents edge from passing through another node

Bug: #3392 Signed-off-by: Florian ROUËNÉ <florian.rouene@obeosoft.com>
eclipse-sirius · May 15, 2024 · fa04e99 · fa04e99
1 parent c7da1fc
commit fa04e99
Show file tree

Hide file tree

Showing 17 changed files with 408 additions and 159 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -74,6 +74,7 @@ Accordingly, `IObjectSearchService` and `IIdentityService` now handle actual `IE
 - Move to docker compose v2 for our GitHub Actions workflow.
 - [test] Switch to ArchUnit 1.3.0 to restore architectural tests which were failing recently
 - https://github.com/eclipse-sirius/sirius-web/issues/3277[#3277] [gantt] Move to @ObeoNetwork/gantt-task-react 0.4.9 to benefit for enhancements
+- https://github.com/eclipse-sirius/sirius-web/issues/3392[#3392] [diagrams] Add dependency to @tisoap/react-flow-smart-edge 3.0.0
 
 === Bug fixes
 
@@ -161,6 +162,7 @@ They still support returning an `java.time.Instant` object directly.
 - https://github.com/eclipse-sirius/sirius-web/issues/3453[#3453] [diagram] Memoizing edges and nodes style
 - https://github.com/eclipse-sirius/sirius-web/issues/3450[#3450] [diagram] Add `calculateCustomNodeBorderNodePosition` method to position border nodes according to the real layout of the custom node
 - https://github.com/eclipse-sirius/sirius-web/issues/3460[#3460] [diagram] Use _arrange layout direction_ properties during edge handle position computing
+- https://github.com/eclipse-sirius/sirius-web/issues/3392[#3392] [diagram] Prevent edge from passing through another node
 
 == v2024.3.0
 

diff --git a/integration-tests/cypress/e2e/project/diagrams/edges.cy.ts b/integration-tests/cypress/e2e/project/diagrams/edges.cy.ts
@@ -42,8 +42,8 @@ describe('Diagram - edges', () => {
           explorer.expand('Entity1 Node');
           details.openReferenceWidgetOptions('Reused Child Node Descriptions');
           details.selectReferenceWidgetOption('Entity2 Node');
-          details.getTextField('Default Width Expression').type('300{enter}');
-          details.getTextField('Default Height Expression').type('300{enter}');
+          details.getTextField('Default Width Expression').type('290{enter}');
+          details.getTextField('Default Height Expression').type('290{enter}');
         });
       })
     );
@@ -83,7 +83,9 @@ describe('Diagram - edges', () => {
           .eq(0)
           .invoke('attr', 'd')
           .then((dValue) => {
-            expect(diagram.roundSvgPathData(dValue ?? '')).to.equal('M150.00L150.00Q150.00L88.00Q83.00L83.00L83.00');
+            expect(diagram.roundSvgPathData(dValue ?? '')).to.equal(
+              'M140.00L140.00L140.00L120.00L100.00L80.00L80.00L80.00'
+          );
           });
       });
     });

diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/diagrams/frontend/sirius-components-diagrams/package.json b/packages/diagrams/frontend/sirius-components-diagrams/package.json
@@ -34,6 +34,7 @@
     "@eclipse-sirius/sirius-components-core": "*",
     "@material-ui/core": "4.12.4",
     "@material-ui/icons": "4.11.3",
+    "@tisoap/react-flow-smart-edge": "3.0.0",
     "elkjs": "0.8.2",
     "graphql": "16.8.0",
     "html-to-image": "1.11.11",

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/converter/convertDiagram.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/converter/convertDiagram.ts
@@ -23,7 +23,7 @@ import {
   ListLayoutStrategy,
 } from '../graphql/subscription/nodeFragment.types';
 import { Diagram, EdgeLabel, NodeData } from '../renderer/DiagramRenderer.types';
-import { MultiLabelEdgeData } from '../renderer/edge/MultiLabelEdge.types';
+import { MultiLabelEdgeData } from '../renderer/edge/MultiLabelEdgeWrapper.types';
 import { RawDiagram } from '../renderer/layout/layout.types';
 import { computeBorderNodeExtents, computeBorderNodePositions } from '../renderer/layout/layoutBorderNodes';
 import { layoutHandles } from '../renderer/layout/layoutHandles';

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/DiagramRenderer.types.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/DiagramRenderer.types.ts
@@ -14,7 +14,7 @@
 import { Edge, Node } from 'reactflow';
 import { GQLNodeDescription } from '../graphql/query/nodeDescriptionFragment.types';
 import { GQLDiagramRefreshedEventPayload } from '../graphql/subscription/diagramEventSubscription.types';
-import { MultiLabelEdgeData } from './edge/MultiLabelEdge.types';
+import { MultiLabelEdgeData } from './edge/MultiLabelEdgeWrapper.types';
 import { ConnectionHandle } from './handles/ConnectionHandles.types';
 import { DiagramNodeType } from './node/NodeTypes.types';
 

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeLayout.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeLayout.ts
@@ -22,7 +22,7 @@ import {
   GetUpdatedConnectionHandlesParameters,
   NodeCenter,
 } from './EdgeLayout.types';
-import { isDescendantOf } from '../layout/layoutNode';
+import { isDescendantOf, isDescendantOfId } from '../layout/layoutNode';
 
 export const getUpdatedConnectionHandles: GetUpdatedConnectionHandlesParameters = (
   sourceNode,
@@ -109,7 +109,9 @@ const getParameters: GetParameters = (movingNode, nodeA, nodeB, visiblesNodes, l
   }
   const horizontalDifference = Math.abs(centerA.x - centerB.x);
   const verticalDifference = Math.abs(centerA.y - centerB.y);
-  const isDescendant = isDescendantOf(nodeB, nodeA, (nodeId) => visiblesNodes.find((node) => node.id === nodeId));
+  const isDescendant = nodeA.data.isBorderNode
+    ? isDescendantOfId(nodeA.parentNode ?? '', nodeB, (nodeId) => visiblesNodes.find((node) => node.id === nodeId))
+    : isDescendantOf(nodeB, nodeA, (nodeId) => visiblesNodes.find((node) => node.id === nodeId));
   let position: Position;
   if (isVerticalLayoutDirection(layoutDirection)) {
     if (isDescendant) {

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeTypes.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeTypes.ts
@@ -12,8 +12,8 @@
  *******************************************************************************/
 
 import { DiagramEdgeTypes } from './EdgeTypes.types';
-import { MultiLabelEdge } from './MultiLabelEdge';
+import { MultiLabelEdgeWrapper } from './MultiLabelEdgeWrapper';
 
 export const edgeTypes: DiagramEdgeTypes = {
-  multiLabelEdge: MultiLabelEdge,
+  multiLabelEdge: MultiLabelEdgeWrapper,
 };
diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/MultiLabelEdge.tsx b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/MultiLabelEdge.tsx
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2023, 2024 Obeo.
+ * Copyright (c) 2024 Obeo.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v2.0
  * which accompanies this distribution, and is available at
@@ -10,18 +10,14 @@
  * Contributors:
  *     Obeo - initial API and implementation
  *******************************************************************************/
+
 import { getCSSColor } from '@eclipse-sirius/sirius-components-core';
 import { Theme, useTheme } from '@material-ui/core/styles';
-import { memo, useContext, useMemo } from 'react';
-import { BaseEdge, EdgeLabelRenderer, EdgeProps, Node, Position, getSmoothStepPath, useStoreApi } from 'reactflow';
-import { NodeTypeContext } from '../../contexts/NodeContext';
-import { NodeTypeContextValue } from '../../contexts/NodeContext.types';
-import { NodeData } from '../DiagramRenderer.types';
+import { memo, useMemo } from 'react';
+import { BaseEdge, EdgeLabelRenderer, Position } from 'reactflow';
 import { Label } from '../Label';
-import { DiagramNodeType } from '../node/NodeTypes.types';
 import { DiagramElementPalette } from '../palette/DiagramElementPalette';
-import { getHandleCoordinatesByPosition } from './EdgeLayout';
-import { MultiLabelEdgeData } from './MultiLabelEdge.types';
+import { MultiLabelEdgeProps } from './MultiLabelEdge.types';
 
 const multiLabelEdgeStyle = (
   theme: Theme,
@@ -58,104 +54,33 @@ const getTranslateFromHandlePositon = (position: Position) => {
 export const MultiLabelEdge = memo(
   ({
     id,
-    source,
-    target,
     data,
     style,
     markerEnd,
     markerStart,
     selected,
     sourcePosition,
     targetPosition,
-    sourceHandleId,
-    targetHandleId,
-  }: EdgeProps<MultiLabelEdgeData>) => {
+    sourceX,
+    sourceY,
+    targetX,
+    targetY,
+    edgeCenterX,
+    edgeCenterY,
+    svgPathString,
+  }: MultiLabelEdgeProps) => {
     const { beginLabel, endLabel, label, faded } = data || {};
     const theme = useTheme();
-    const { nodeLayoutHandlers } = useContext<NodeTypeContextValue>(NodeTypeContext);
-
-    const { nodeInternals } = useStoreApi().getState();
-
-    const sourceNode = nodeInternals.get(source);
-    const targetNode = nodeInternals.get(target);
 
     const edgeStyle = useMemo(() => multiLabelEdgeStyle(theme, style, selected, faded), [style, selected, faded]);
     const sourceLabelTranslation = useMemo(() => getTranslateFromHandlePositon(sourcePosition), [sourcePosition]);
     const targetLabelTranslation = useMemo(() => getTranslateFromHandlePositon(targetPosition), [targetPosition]);
 
-    if (!sourceNode || !targetNode) {
-      return null;
-    }
-
-    const sourceLayoutHandler = nodeLayoutHandlers.find((nodeLayoutHandler) =>
-      nodeLayoutHandler.canHandle(sourceNode as Node<NodeData, DiagramNodeType>)
-    );
-    const targetLayoutHandler = nodeLayoutHandlers.find((nodeLayoutHandler) =>
-      nodeLayoutHandler.canHandle(targetNode as Node<NodeData, DiagramNodeType>)
-    );
-
-    let { x: sourceX, y: sourceY } = getHandleCoordinatesByPosition(
-      sourceNode,
-      sourcePosition,
-      sourceHandleId ?? '',
-      sourceLayoutHandler?.calculateCustomNodeEdgeHandlePosition
-    );
-    let { x: targetX, y: targetY } = getHandleCoordinatesByPosition(
-      targetNode,
-      targetPosition,
-      targetHandleId ?? '',
-      targetLayoutHandler?.calculateCustomNodeEdgeHandlePosition
-    );
-
-    // trick to have the source of the edge positioned at the very border of a node
-    // if the edge has a marker, then only the marker need to touch the node
-    const handleSourceRadius = markerStart == undefined || markerStart.includes('None') ? 2 : 3;
-    switch (sourcePosition) {
-      case Position.Right:
-        sourceX = sourceX + handleSourceRadius;
-        break;
-      case Position.Left:
-        sourceX = sourceX - handleSourceRadius;
-        break;
-      case Position.Top:
-        sourceY = sourceY - handleSourceRadius;
-        break;
-      case Position.Bottom:
-        sourceY = sourceY + handleSourceRadius;
-        break;
-    }
-    // trick to have the target of the edge positioned at the very border of a node
-    // if the edge has a marker, then only the marker need to touch the node
-    const handleTargetRadius = markerEnd == undefined || markerEnd.includes('None') ? 2 : 3;
-    switch (targetPosition) {
-      case Position.Right:
-        targetX = targetX + handleTargetRadius;
-        break;
-      case Position.Left:
-        targetX = targetX - handleTargetRadius;
-        break;
-      case Position.Top:
-        targetY = targetY - handleTargetRadius;
-        break;
-      case Position.Bottom:
-        targetY = targetY + handleTargetRadius;
-        break;
-    }
-
-    const [edgePath, labelX, labelY] = getSmoothStepPath({
-      sourceX,
-      sourceY,
-      sourcePosition,
-      targetX,
-      targetY,
-      targetPosition,
-    });
-
     return (
       <>
         <BaseEdge
           id={id}
-          path={edgePath}
+          path={svgPathString}
           style={edgeStyle}
           markerEnd={selected ? `${markerEnd?.slice(0, markerEnd.length - 1)}--selected)` : markerEnd}
           markerStart={selected ? `${markerStart?.slice(0, markerStart.length - 1)}--selected)` : markerStart}
@@ -171,7 +96,12 @@ export const MultiLabelEdge = memo(
             />
           )}
           {label && (
-            <Label diagramElementId={id} transform={`translate(${labelX}px,${labelY}px)`} label={label} faded={false} />
+            <Label
+              diagramElementId={id}
+              transform={`translate(${edgeCenterX}px,${edgeCenterY}px)`}
+              label={label}
+              faded={false}
+            />
           )}
           {endLabel && (
             <Label