react-flow-smart-edge icon indicating copy to clipboard operation
react-flow-smart-edge copied to clipboard

Smart Edge wrong behaviour on Sub Flows

Open spaceymonk opened this issue 2 years ago • 5 comments

First of all, I thank you for your great work.

The issue is drawing edges in subflows is wrong. In the below figures parentNode of the smaller nodes is the WHILE node.

Expected behaviour: image It should be drawn like this for all positions inside the parent node.

Actual Result: image After moving a little bit on one of the nodes, the edge was drawn wrong.

Maybe something similar to #28 can solve this.

spaceymonk avatar Jun 09 '22 08:06 spaceymonk

Hey @spaceymonk , thanks for reporting it! Do you have a minimal example on CodeSandbox or a minimal GitHub repo where the error happens? This would help me debug the issue

My educated guess is that sub flows are treated as nodes and the path-finding algorithm tries to include them on it's calculations

tisoap avatar Jun 10 '22 01:06 tisoap

Sure, you can look at this sandbox.

I think so, maybe parent nodes can be somewhat ignored during calculating the path.

spaceymonk avatar Jun 10 '22 07:06 spaceymonk

@spaceymonk I just launched version 2.0.0 of this library, that exposes a getSmartEdge function instead of factories, check the new README. It accepts a nodes argument, so you could filter out sub-flows before calling this function.

I still need to consider if it's worth to have a sub flow filter on the library itself, so I'll leave this issue open

tisoap avatar Jun 12 '22 19:06 tisoap

As a workaround you could filter the array of nodes passed to the getSmartEdge function:

const getSmartEdgeResponse = getSmartEdge({
  sourcePosition,
  targetPosition,
  sourceX,
  sourceY,
  targetX,
  targetY,
  nodes: nodes.filter(node => node.parentNode),
  options: edgeOptions,
})

jeromedg-dlh avatar Jun 21 '22 08:06 jeromedg-dlh

Thank you for your efforts, after upgrading to the new version and filtering the parent nodes, it worked like a charm. About your question,

@spaceymonk I just launched version 2.0.0 of this library, that exposes a getSmartEdge function instead of factories, check the new README. It accepts a nodes argument, so you could filter out sub-flows before calling this function.

I still need to consider if it's worth to have a sub flow filter on the library itself, so I'll leave this issue open

After supplying the node list, I think the expected behaviour is to correctly handle sub-flow connections.

spaceymonk avatar Jun 21 '22 10:06 spaceymonk