Skip to content

Respect explicit node x/y positions in fixed-arrangement Sankey #7826

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.

Sign up for GitHub

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

Open
kyo5uke wants to merge 2 commits into
base: master
Choose a base branch
from
+18 −9
Open

Respect explicit node x/y positions in fixed-arrangement Sankey #7826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf95567
Fix explicit node positions in fixed-arrangement Sankey
kyo5uke Jun 5, 2026
86084f9
Add draftlog for #7826
kyo5uke Jun 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/7826_fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Respect explicit `node.x`/`node.y` positions in Sankey traces that contain unlinked or grouped nodes, by indexing the forced positions by each node's `pointNumber` instead of its array position [[#7826](https://github.com/plotly/plotly.js/pull/7826)]
26 changes: 17 additions & 9 deletions src/traces/sankey/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,23 @@ function sankeyModel(layout, d, traceIndex) {

// Force node position
if(trace.node.x.length && trace.node.y.length) {
for(i = 0; i < Math.min(trace.node.x.length, trace.node.y.length, graph.nodes.length); i++) {
if(trace.node.x[i] && trace.node.y[i]) {
var pos = [trace.node.x[i] * width, trace.node.y[i] * height];
graph.nodes[i].x0 = pos[0] - nodeThickness / 2;
graph.nodes[i].x1 = pos[0] + nodeThickness / 2;

var nodeHeight = graph.nodes[i].y1 - graph.nodes[i].y0;
graph.nodes[i].y0 = pos[1] - nodeHeight / 2;
graph.nodes[i].y1 = pos[1] + nodeHeight / 2;
// graph.nodes omits unlinked nodes and may have transient group
// children unshifted to its front, so its order does not match
// trace.node.x/y. Index the explicit positions by each node's
// original pointNumber rather than by array position.
Comment on lines +247 to +250

@camdecoster camdecoster Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// graph.nodes omits unlinked nodes and may have transient group
// children unshifted to its front, so its order does not match
// trace.node.x/y. Index the explicit positions by each node's
// original pointNumber rather than by array position.
// Index the positions by `pointNumber` to avoid issues with
// array order diverging from node order

for(i = 0; i < graph.nodes.length; i++) {
var forcedNode = graph.nodes[i];
if(forcedNode.partOfGroup) continue;

var p = forcedNode.pointNumber;
if(trace.node.x[p] && trace.node.y[p]) {

@camdecoster camdecoster Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 0 coords will get dropped with the existing check (preexisting bug). Let's update it. This should take care of null and undefined.

Suggested change
if(trace.node.x[p] && trace.node.y[p]) {
if(trace.node.x[p] != null && trace.node.y[p] != null) {

var pos = [trace.node.x[p] * width, trace.node.y[p] * height];
forcedNode.x0 = pos[0] - nodeThickness / 2;
forcedNode.x1 = pos[0] + nodeThickness / 2;

var nodeHeight = forcedNode.y1 - forcedNode.y0;
forcedNode.y0 = pos[1] - nodeHeight / 2;
forcedNode.y1 = pos[1] + nodeHeight / 2;
}
}
if(trace.arrangement === 'snap') {
Expand Down
Loading