Skip to content

Commit

Permalink
[3651] Improve isDescendantOf used in diagram
Browse files Browse the repository at this point in the history
Bug: #3651
Signed-off-by: Michaël Charfadi <[email protected]>
  • Loading branch information
mcharfadi authored and sbegaudeau committed Jun 20, 2024
1 parent 822adbe commit 1eab033
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 88 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ image:doc/screenshots/insideLabelPositions.png[Inside label positions, 70%]
- https://github.com/eclipse-sirius/sirius-web/issues/3634[#3634] [sirius-web] Simplifying the contribution to the GraphQL subscription of the diagram for custom nodes
- https://github.com/eclipse-sirius/sirius-web/issues/3656[#3656] [core] Add the ability to customize the GraphQL type resolver
- https://github.com/eclipse-sirius/sirius-web/issues/3645[#3645] [core] Revert earlier change made in #3595 which caused regressions
- https://github.com/eclipse-sirius/sirius-web/issues/3651[#3651] [diagram] Improve performances when interacting with the diagram, especially when dragging a node


== v2024.5.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,15 @@ export const convertDiagram = (
nodes,
edges,
};

const nodeInternals = new Map();
nodes.forEach((node) => {
nodeInternals.set(node.id, node);
});

computeBorderNodeExtents(rawDiagram.nodes);
computeBorderNodePositions(rawDiagram.nodes);
layoutHandles(rawDiagram, diagramDescription);
layoutHandles(rawDiagram, diagramDescription, nodeInternals);

return {
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const useConnector = (): UseConnectorValue => {
} = useContext<ConnectorContextValue>(ConnectorContext);

const reactFlowInstance = useReactFlow<NodeData, EdgeData>();
const { getNodes, getNode, setEdges } = reactFlowInstance;
const { getNode, setEdges } = reactFlowInstance;

const theme = useTheme();
const { hideDiagramElementPalette } = useDiagramElementPalette();
Expand Down Expand Up @@ -110,7 +110,7 @@ export const useConnector = (): UseConnectorValue => {
const { targetPosition, sourcePosition } = getEdgeParameters(
sourceNode,
targetNode,
getNodes(),
store.getState().nodeInternals,
diagramDescription.arrangeLayoutDirection
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import { gql, useMutation } from '@apollo/client';
import { useMultiToast } from '@eclipse-sirius/sirius-components-core';
import { useCallback, useContext, useEffect } from 'react';
import { Node, NodeDragHandler, XYPosition, useReactFlow } from 'reactflow';
import { Node, NodeDragHandler, XYPosition, useReactFlow, useStoreApi } from 'reactflow';
import { DiagramContext } from '../../contexts/DiagramContext';
import { DiagramContextValue } from '../../contexts/DiagramContext.types';
import { useDiagramDescription } from '../../contexts/useDiagramDescription';
Expand Down Expand Up @@ -133,13 +133,16 @@ export const useDropNode = (): UseDropNodeValue => {
const onDropNode = useDropNodeMutation();
const { getNodes, getIntersectingNodes, screenToFlowPosition } = useReactFlow<NodeData, EdgeData>();
const { setNodes } = useStore();
const storeApi = useStoreApi();

const getNodeById: (string) => Node | undefined = (id: string) => getNodes().find((n) => n.id === id);
const getNodeById = (id: string) => storeApi.getState().nodeInternals.get(id);

const getDraggableNode = (node: Node<NodeData>): Node<NodeData> => {
const parentNode = getNodeById(node.parentNode);
if (parentNode && isListData(parentNode) && !parentNode.data.areChildNodesDraggable) {
return getDraggableNode(parentNode);
if (node.parentNode) {
const parentNode = getNodeById(node.parentNode);
if (parentNode && isListData(parentNode) && !parentNode.data.areChildNodesDraggable) {
return getDraggableNode(parentNode);
}
}
return node;
};
Expand All @@ -156,7 +159,10 @@ export const useDropNode = (): UseDropNodeValue => {
(entry) => entry.droppedNodeDescriptionId === (computedNode as Node<NodeData>).data.descriptionId
);
const compatibleNodes = getNodes()
.filter((candidate) => !candidate.hidden && !isDescendantOf(computedNode, candidate, getNodeById))
.filter(
(candidate) =>
!candidate.hidden && !isDescendantOf(computedNode, candidate, storeApi.getState().nodeInternals)
)
.filter((candidate) =>
dropDataEntry?.droppableOnNodeTypes.includes((candidate as Node<NodeData>).data.descriptionId)
)
Expand Down Expand Up @@ -197,7 +203,9 @@ export const useDropNode = (): UseDropNodeValue => {
const intersections = getIntersectingNodes(draggedNode).filter((intersectingNode) => !intersectingNode.hidden);
const newParentId =
[...intersections]
.filter((intersectingNode) => !isDescendantOf(draggedNode, intersectingNode, getNodeById))
.filter(
(intersectingNode) => !isDescendantOf(draggedNode, intersectingNode, storeApi.getState().nodeInternals)
)
.sort((n1, n2) => getNodeDepth(n2, intersections) - getNodeDepth(n1, intersections))[0]?.id || null;

const targetNode = getNodes().find((node) => node.data.isDropNodeTarget) || null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { HandleElement, Position, XYPosition, internalsSymbol } from 'reactflow'
import { BorderNodePosition } from '../DiagramRenderer.types';
import { ConnectionHandle } from '../handles/ConnectionHandles.types';
import { isDescendantOf, isSiblingOrDescendantOf } from '../layout/layoutNode';
import { horizontalLayoutDirectionGap, verticalLayoutDirectionGap } from '../layout/layoutParams';
import {
GetEdgeParameters,
GetEdgeParametersWhileMoving,
Expand All @@ -24,7 +25,6 @@ import {
GetUpdatedConnectionHandlesParameters,
NodeCenter,
} from './EdgeLayout.types';
import { verticalLayoutDirectionGap, horizontalLayoutDirectionGap } from '../layout/layoutParams';

export const getUpdatedConnectionHandles: GetUpdatedConnectionHandlesParameters = (
sourceNode,
Expand Down Expand Up @@ -62,21 +62,21 @@ export const getEdgeParametersWhileMoving: GetEdgeParametersWhileMoving = (
movingNode,
source,
target,
visiblesNodes,
nodeInternals,
layoutDirection
) => {
const { position: sourcePosition } = getParameters(movingNode, source, target, visiblesNodes, layoutDirection);
const { position: targetPosition } = getParameters(movingNode, target, source, visiblesNodes, layoutDirection);
const { position: sourcePosition } = getParameters(movingNode, source, target, nodeInternals, layoutDirection);
const { position: targetPosition } = getParameters(movingNode, target, source, nodeInternals, layoutDirection);

return {
sourcePosition,
targetPosition,
};
};

export const getEdgeParameters: GetEdgeParameters = (source, target, visiblesNodes, layoutDirection) => {
const { position: sourcePosition } = getParameters(null, source, target, visiblesNodes, layoutDirection);
const { position: targetPosition } = getParameters(null, target, source, visiblesNodes, layoutDirection);
export const getEdgeParameters: GetEdgeParameters = (source, target, nodeInternals, layoutDirection) => {
const { position: sourcePosition } = getParameters(null, source, target, nodeInternals, layoutDirection);
const { position: targetPosition } = getParameters(null, target, source, nodeInternals, layoutDirection);

return {
sourcePosition,
Expand Down Expand Up @@ -107,11 +107,9 @@ const computeBorderNodeHandlePosition = (
}
};

const getParameters: GetParameters = (movingNode, nodeA, nodeB, visiblesNodes, layoutDirection) => {
const getParameters: GetParameters = (movingNode, nodeA, nodeB, nodeInternals, layoutDirection) => {
if (nodeA.data.isBorderNode) {
const isInside = isSiblingOrDescendantOf(nodeA, nodeB, (nodeId) =>
visiblesNodes.find((node) => node.id === nodeId)
);
const isInside = isSiblingOrDescendantOf(nodeA, nodeB, nodeInternals);
return {
position: computeBorderNodeHandlePosition(nodeA.data.borderNodePosition, isInside),
};
Expand All @@ -123,7 +121,7 @@ const getParameters: GetParameters = (movingNode, nodeA, nodeB, visiblesNodes, l
y: (movingNode.positionAbsolute?.y ?? 0) + (nodeA.height ?? 0) / 2,
};
} else {
centerA = getNodeCenter(nodeA, visiblesNodes);
centerA = getNodeCenter(nodeA, nodeInternals);
}

let centerB: NodeCenter;
Expand All @@ -133,11 +131,11 @@ const getParameters: GetParameters = (movingNode, nodeA, nodeB, visiblesNodes, l
y: (movingNode.positionAbsolute?.y ?? 0) + (nodeB.height ?? 0) / 2,
};
} else {
centerB = getNodeCenter(nodeB, visiblesNodes);
centerB = getNodeCenter(nodeB, nodeInternals);
}
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 = isDescendantOf(nodeB, nodeA, nodeInternals);
let position: Position;
if (isVerticalLayoutDirection(layoutDirection)) {
if (Math.abs(centerA.y - centerB.y) < verticalLayoutDirectionGap) {
Expand Down Expand Up @@ -175,25 +173,28 @@ const getParameters: GetParameters = (movingNode, nodeA, nodeB, visiblesNodes, l
};
};

export const getNodeCenter: GetNodeCenter = (node, visiblesNodes) => {
export const getNodeCenter: GetNodeCenter = (node, nodeInternals) => {
if (node.positionAbsolute?.x && node.positionAbsolute?.y) {
return {
x: (node.positionAbsolute?.x ?? 0) + (node.width ?? 0) / 2,
y: (node.positionAbsolute?.y ?? 0) + (node.height ?? 0) / 2,
};
} else {
let parentNode = visiblesNodes.find((nodeParent) => nodeParent.id === node.parentNode);
let position = {
x: (node.position?.x ?? 0) + (node.width ?? 0) / 2,
y: (node.position?.y ?? 0) + (node.height ?? 0) / 2,
};
while (parentNode) {
position = {
x: position.x + (parentNode.position?.x ?? 0),
y: position.y + (parentNode.position?.y ?? 0),
};
let parentNodeId = parentNode.parentNode ?? '';
parentNode = visiblesNodes.find((nodeParent) => nodeParent.id === parentNodeId);

if (node.parentNode) {
let parentNode = nodeInternals.get(node.parentNode);
while (parentNode) {
position = {
x: position.x + (parentNode.position?.x ?? 0),
y: position.y + (parentNode.position?.y ?? 0),
};
let parentNodeId = parentNode.parentNode ?? '';
parentNode = nodeInternals.get(parentNodeId);
}
}
return position;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* Obeo - initial API and implementation
*******************************************************************************/

import { HandleElement, Node, NodePositionChange, Position, XYPosition } from 'reactflow';
import { HandleElement, Node, NodeInternals, NodePositionChange, Position, XYPosition } from 'reactflow';
import { NodeData } from '../DiagramRenderer.types';
import { ConnectionHandle } from '../handles/ConnectionHandles.types';

Expand All @@ -38,14 +38,14 @@ export type GetEdgeParametersWhileMoving = (
movingNode: NodePositionChange,
source: Node<NodeData>,
target: Node<NodeData>,
visiblesNodes: Node<NodeData>[],
nodeInternals: NodeInternals,
layoutDirection: string
) => EdgeParameters;

export type GetEdgeParameters = (
source: Node<NodeData>,
target: Node<NodeData>,
visiblesNodes: Node<NodeData>[],
nodeInternals: NodeInternals,
layoutDirection: string
) => EdgeParameters;

Expand All @@ -58,15 +58,15 @@ export type GetParameters = (
movingNode: NodePositionChange | null,
nodeA: Node<NodeData>,
nodeB: Node<NodeData>,
visiblesNodes: Node<NodeData>[],
nodeInternals: NodeInternals,
layoutDirection: string
) => Parameters;

export interface Parameters {
position: Position;
}

export type GetNodeCenter = (node: Node<NodeData>, visiblesNodes: Node<NodeData>[]) => NodeCenter;
export type GetNodeCenter = (node: Node<NodeData>, nodeInternals: NodeInternals) => NodeCenter;

export interface NodeCenter {
x: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,22 @@
* Obeo - initial API and implementation
*******************************************************************************/
import { useCallback } from 'react';
import { Node, NodeChange, NodePositionChange, getConnectedEdges } from 'reactflow';
import { Node, NodeChange, NodePositionChange, getConnectedEdges, useStoreApi } from 'reactflow';
import { useDiagramDescription } from '../../contexts/useDiagramDescription';
import { useStore } from '../../representation/useStore';
import { NodeData } from '../DiagramRenderer.types';
import { getEdgeParametersWhileMoving, getUpdatedConnectionHandles } from '../edge/EdgeLayout';
import { DiagramNodeType } from '../node/NodeTypes.types';
import { ConnectionHandle } from './ConnectionHandles.types';
import { UseHandleChangeValue } from './useHandleChange.types';
import { useDiagramDescription } from '../../contexts/useDiagramDescription';

const isNodePositionChange = (change: NodeChange): change is NodePositionChange =>
change.type === 'position' && typeof change.dragging === 'boolean' && change.dragging;

export const useHandleChange = (): UseHandleChangeValue => {
const { getEdges } = useStore();
const { diagramDescription } = useDiagramDescription();
const storeApi = useStoreApi();

const applyHandleChange = useCallback(
(changes: NodeChange[], nodes: Node<NodeData, DiagramNodeType>[]): Node<NodeData, DiagramNodeType>[] => {
Expand All @@ -44,7 +45,7 @@ export const useHandleChange = (): UseHandleChangeValue => {
nodeDraggingChange,
sourceNode,
targetNode,
nodes,
storeApi.getState().nodeInternals,
diagramDescription.arrangeLayoutDirection
);
const nodeSourceConnectionHandle: ConnectionHandle | undefined = sourceNode.data.connectionHandles.find(
Expand Down
Loading

0 comments on commit 1eab033

Please sign in to comment.