Stable sorting links for better parallel edge handling

[monfera]

Currently, link Y ordering is the result of non-stable sorting on both the source and target nodes, with the result that multi-edges may unnecessarily cross each other. By turning it into a stable short, via the use of an arbitrary but shareable (between source/target) tie breaker such as the original index of a link, we get better results.

The dataset is from a data URL of Washington Post - Fallen from the skies: drone crashes database which shows the utility of multi-edges (parallel links) for things like hover interactions and/or focus+context interactions. Attribute based coloring is yet another possibility.

All is not perfect though - the edges between the same source and target overlap slightly, depending on the Y offset between the anchor points and where we are on the line. This overlap may help or hinder the design, depending on goals, color and other styling.

[curran]

Huge improvement! Nice work.

[monfera]

Examples for multi-edges:

• drag/drop node to show overlapping artifact dependent on thickness and vertical distance - i.e. stable sorting is a useful step with low impact, and we're thinking up a future update that would avoid the overlap
• hover items showing fine-grained breakdown (obsolete plotly code version, just an extreme case of granularity)

[wiesson]

Any updates to this PR?

[monfera]

@wiesson I have no information on this, and there's a related other PR #24 so, for the time being, we made a friendly fork with the thought of eventually syncing up. As such, it got published on npm under the '@plotly' scope, it can be installed as npm install @plotly/d3-sankey, or by putting dependencies: {"@plotly/d3-sankey": "^0.5.0"} in package.json.

This example shows the effect of both of the PRs mentioned and has crosscutting plotly.js features e.g. tooltip, and the force layout for easier node rearrangement.

I think this specific PR is non-controversial, but the other PR we merged in the forked version is worth pondering, see the discussion

[monfera]

@arankek cc-ing you in case you have more info for @wiesson

[monfera]

... due to cleanup of src/sankey.js by @mbostock the merge conflict needs to be resolved first, I'm glad to do it if useful

[mbostock]

I’m on it. Going to rename to link.index and populate node.index, too.

[mbostock]

Merged!