Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smart Edge wrong behaviour on Sub Flows #32

Open
spaceymonk opened this issue Jun 9, 2022 · 5 comments
Open

Smart Edge wrong behaviour on Sub Flows #32

spaceymonk opened this issue Jun 9, 2022 · 5 comments
Labels
bug Something isn't working pinned This should not be closed by bots on inactivity

Comments

@spaceymonk
Copy link

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
Copy link
Owner

tisoap commented Jun 10, 2022

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 tisoap added bug Something isn't working pinned This should not be closed by bots on inactivity labels Jun 10, 2022
@tisoap tisoap changed the title Connection in subflows fails (reactflow v10) Smart Edge wrong behaviour on Sub Flows Jun 10, 2022
@spaceymonk
Copy link
Author

Sure, you can look at this sandbox.

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

@tisoap
Copy link
Owner

tisoap commented Jun 12, 2022

@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
Copy link

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
Copy link
Author

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.

Repository owner locked and limited conversation to collaborators Jun 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working pinned This should not be closed by bots on inactivity
Projects
None yet
Development

No branches or pull requests

3 participants