Sankey: add attributes node.(x|y)

[antoinerg]

This closes parts of #2520.

It adds attributes node.(x|y) to specify the position of nodes.

Upon reordering the nodes by dragging them with the mouse, we update those attributes to persist the state. This allow saving the current state of the figure to an image whereas it used to always export the initial position of the nodes.

This PR also greatly improves our Sankey tests by testing drag operations for every arrangement! This allowed me to catch a regression as discussed in #3583 (comment) .

[antoinerg]

One thing I dislike about the current branch is that ky, the ratio between the height in pixel of a node and its value, is computed by the underlying Sankey generator. So although the user can position the nodes by specifying node.(x|y), there's no way of knowing how tall the nodes will be. This makes it impossible to decide ahead of times what would be good values of node.y to prevent nodes from overlapping vertically...

I am thinking we might want to add a new attribute node.scale 🤔 What do you think?

cc @alexcjohnson @plotly/plotly_js

[alexcjohnson]

When you drag nodes around by GUI, the others slide around to avoid overlaps. What if we did the same here? You provide positions but then we slide them around, keeping the ordering fixed, to avoid overlaps. It would still have some weird aspects, like if you're on a column that doesn't span the whole height and you want the nodes clustered in certain places you'd have to artificially squish them and trust us to sort it out.

node.scale could still be an interesting idea - though it's not just nodes, it applies to links too. But that seems like a separate feature, and in any case we don't want to allow overlapping nodes so we'll have to reserve the right to modify any positioning the user provides.

[antoinerg]

Thank you @alexcjohnson for the quick reply! I will give this a bit more thought. In the meantime, here are a few comments:

in any case we don't want to allow overlapping nodes

If the arrangement is freeform or perpendicular, we actually allow nodes to overlap. In fact, dragging in freeform allow nodes to be anywhere! This is why I set out to support arbitrary positioning via node.(x|y) to be on par with what can be done with the mouse.

If we decide to get rid of overlapping nodes, I could definitely try to implement your suggestion:

you provide positions but then we slide them around, keeping the ordering fixed, to avoid overlaps.

Finally,

node.scale could still be an interesting idea - though it's not just nodes, it applies to links too.

You're right: it should be a top-level attribute (ie. scale instead of node.scale).

[alexcjohnson]

Ah OK, I guess I hadn't seen the interaction modes that allow overlaps to persist. In that case those should still be allowed, and should play nice with explicit positioning. But seems to me the default mode should be to respect explicit positioning but then push apart overlaps.

[antoinerg]

Issue #3563 is also present for arrangement freeform and perpendicular (and hence in master) but it is fixed in the current PR and now has proper tests.

[antoinerg]

But seems to me the default mode should be to respect explicit positioning but then push apart overlaps.

@alexcjohnson After the nodes are pushed apart in snap mode (which is the default), should I update the node.(x|y) to reflect the new positions of the nodes?

[antoinerg]

@alexcjohnson After the nodes are pushed apart in snap mode (which is the default), should I update the node.(x|y) to reflect the new positions of the nodes?

I elected to not update node.(x|y) if the user specified them explicitly. This way, the user can switch from freeform to snap to freeform again.

[chriddyp]

Will this work with uirevision?

[antoinerg]

Will this work with uirevision?

@chriddyp it now does with commit 8ddfe7e ! Thanks for suggesting the improvement! 😸

[etpinard]

@antoinerg what's the status on this one? Are planning on adding anything else to this PR?

[etpinard]

but it is fixed in the current PR and now has proper tests.
Will this work with uirevision?
@chriddyp it now does with commit 8ddfe7e !

Amazing work 🎉

[etpinard]

Do all the possible values of arrangement have an effect of the node layout when node.x and node.y are set?

[antoinerg]

Do all the possible values of arrangement have an effect of the node layout when node.x and node.y are set?

Short answer: only snap can override node.(x|y)

Long answer:
When node.(x|y) is set, the nodes are forced to sit at the specified locations except in the snap arrangement. When drag and dropping, snap is the only arrangement in which we forbid nodes from overlapping so I made sure we keep honoring this promise.

The goal of this PR was to make it possible to replicate any state accessible via drag and drop via node.(x|y) and hence allowing to export images.

[etpinard]

Ok, thanks for the info!

Could we then make arrangement default to 'freeform' whenever node.(x|y) are set? That way, a user that wants to programmatically tweak the node position via node.(x|y) can do so w/o having to worry about changing the arrangement value.

[antoinerg]

Done in 65c7201

[antoinerg]

@antoinerg what's the status on this one? Are planning on adding anything else to this PR?

I wasn't planning on adding anything else to this PR. It is bigger than I initially anticipated and I don't want to complicate it further at this stage.

[etpinard]

Nicely done 💃 💃