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
256 stars 35 forks source link

Providing support for custom edge label to be displayed #24

Closed NithishG closed 2 years ago

NithishG commented 2 years ago

23

tisoap commented 2 years ago

Hey @NithishG , thanks for your contribution! Unfortunately I can't accept this PR as is, if we are to enable people to add custom foreignObject elements as edge labels, it can't have hard coded elements/values, it needs to be generic enough for everyone else using this package. You call always apply the changes in your fork and use it instead if you're in a hurry

NithishG commented 2 years ago

@tisoap Sorry I didn't get what you meant by it can't have hard coded elements/values. Could you please let me know so that I'll make it as generic as possible.

If you are talking about CustomLabel.tsx file, it's just an example and the file is not under the actual library code but is under stories in order to showcase how one can provide a foreignObject if they want to.

tisoap commented 2 years ago

@NithishG Sorry I didn't realize at first that CustomLabel.tsx was just part of the example. But there's a few problems with this implementation:

  1. The users would need to pass their custom label using the label key inside the edges object array. I'd rather keep both nodes and edges props serializable, which makes it easier to save the graph state to an external service (like a database)
  2. Here we would be rendering whatever custom label as a child of EdgeText, and also passing it as a label prop to it, which is not what EdgeText was designed for
  3. As it stands, by rendering the custom label like {typeof label !== 'string' && label}, we lose the opportunity to pass down props to the custom foreignObject. Take the Edge with Button example from React Flow, how would we implement a button that returns the edge ID that it is from?

I haven't started thinking in an API for this, but I believe our best bet is to add another option for smartEdgeFactory, that lets the user pass a custom JSXElementConstructor that will receive props from the edge (like it's id)

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.