xyflow / xyflow

React Flow | Svelte Flow - Powerful open source libraries for building node-based UIs with React (https://reactflow.dev) or Svelte (https://svelteflow.dev). Ready out-of-the-box and infinitely customizable.
https://xyflow.com
MIT License
22.07k stars 1.46k forks source link

Updatable edges: granular control over whether EdgeAnchor appears #4260

Closed IanLondon closed 1 month ago

IanLondon commented 1 month ago

Please describe the feature that you want to propose

Desired API behavior

edge.updatable (docs) should add the EdgeAnchor to both the target and source ends if true, or to just the designated end if set to "target" or "source". This should override whatever props are set on the ReactFlow component. This would allow a high degree of control: for example, my "derived" edges are not updatable by dragging the ends, but my "flexible" edges are.

Pie in the sky feature request :pie: :sun_behind_small_cloud:

These draggable circle areas aren't my ideal shape for this interaction, I'd prefer a transparent <path> with a larger stroke overlaid on the edge itself so I could grab something like the bottom N units (or bottom N%) of an edge to start dragging it to a different node.

Also, is there any way I can create an "updatable edge handle" with my own component? I found this issue https://github.com/xyflow/xyflow/issues/936 but it looks like the PR just gave the EdgeAnchor an offset and didn't allow a developer to customize the EdgeAnchor component itself.

Related

https://github.com/xyflow/web/issues/384 my request for more docs on existing behavior

https://github.com/xyflow/xyflow/issues/1095 is related but deals with a different issue about positioning the EdgeAnchors.

bcakmakoglu commented 1 month ago

To address some of the things in here

edge.updatable should add the EdgeAnchor to both the target and source ends if true, or to just the designated end if set to "target" or "source".

This is already the case. If edge.updatable is true, both EdgeAnchors will appear etc.

This should override whatever props are set on the ReactFlow component.

This is also already the case, edge.updatable always has precedence over the global edgesUpdatable prop

It feels weird that the onEdgeUpdate callback is the way to toggle "updatable edges". I'm using onEdgesChange for the actual logic of all changes to my edges [...]

onEdgesChange is not a replacement for onEdgeUpdate so I'm not sure where the assumption comes from that you can use one or the other. onEdgesChange is meant to communicate internal changes to the user, like selecting, adding or removing an edge. Updates are not part of the changes that are pushed through onEdgesChange (although it could be argued that these should also go in there ^^)

The reasoning, at least from my point of view, for the requirement of passing an actual onEdgeUpdate handler is that you as a user have to handle the update. ReactFlow doesn't make assumptions about how an edge should be updated, it only communicates to you that these are the params of the update. You can do whatever you want with the params: maybe you want to validate the update first, maybe you want to modify the edge params before the updating, maybe you wanna do something else entirely.

Assuming that onEdgeUpdate was not required and it RF would handle the update for you, I could see people opening Issues just like this one complaining why RF assumes to know how the update should be handled 😅 So imo I think having the users take care of this solves it pretty nicely and I don't quite see where the problem lies with passing a basic handler if that's all you need for your use-case?

Meanwhile edgesUpdatable bool seems to only exist to disable "updatable edges"?

More or less - if you passed an update handler and want to toggle the updatable behavior, you can turn it on and off globally with this prop, although as mentioned previously edge.updatable always has precedence when determining if an edge is updatable.

IanLondon commented 1 month ago

That makes sense, I didn't realize until I started playing with it that the onEdgeUpdate doesn't dispatch any EdgeRemoveChange | EdgeAddChange to onEdgesChange.