tisoap / react-flow-smart-edge

Custom Edge for React Flow that never intersects with other nodes
https://tisoap.github.io/react-flow-smart-edge/
MIT License
242 stars 26 forks source link

Smart Edge wrong behaviour on Sub Flows #32

Open spaceymonk opened 2 years ago

spaceymonk commented 2 years ago

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.

tisoap commented 2 years ago

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

spaceymonk commented 2 years ago

Sure, you can look at this sandbox.

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

tisoap commented 2 years ago

@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

jeromedg-dlh commented 2 years ago

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,
})
spaceymonk commented 2 years ago

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.