Skip to content

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

Open
kyo5uke wants to merge 2 commits into
plotly:masterfrom
kyo5uke:sankey-fixed-node-positions
Open

Respect explicit node x/y positions in fixed-arrangement Sankey#7826
kyo5uke wants to merge 2 commits into
plotly:masterfrom
kyo5uke:sankey-fixed-node-positions

Conversation

@kyo5uke
Copy link
Copy Markdown

@kyo5uke kyo5uke commented Jun 5, 2026

Overview

Fixes the bug reported in #7758: when a Sankey trace specifies explicit node.x / node.y
positions (typically with arrangement: 'fixed'), the coordinates were applied to the wrong
nodes whenever the trace contained an unlinked (isolated) node or grouped nodes. In the issue's
example, right_a — specified at (0.9, 0.05) (top-right) — rendered at the bottom-left.

Root cause

The "Force node position" loop in src/traces/sankey/render.js applied the explicit positions by
array position, assuming graph.nodes[i] corresponds to input node i:

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]) { /* ... graph.nodes[i] ... */ }
}

That assumption does not hold:

  • calc.js skips unlinked nodes when building the node list (if(!linkedNodes[i]) continue;), so
    isolated nodes are absent from graph.nodes.
  • render.js unshifts transient group-child nodes onto the front of graph.nodes.

Either case shifts every later node's array position away from its original input index. In the
issue's data, left_r (index 17) has no links, so it is dropped from graph.nodes; every node
after it is then off by one, and right_a (index 18) receives left_r's coordinate (0.1, 0.82)
— the bottom-left position shown in the report.

Each node already stores its original input index as pointNumber (set in calc.js), so indexing
the explicit positions by pointNumber instead of array position restores the correct mapping.

Change

src/traces/sankey/render.js — iterate over all graph.nodes and look up the explicit position by
the node's own pointNumber:

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]) { /* ... forcedNode ... */ }
}
  • Synthetic group nodes have pointNumber >= nodeCount, so trace.node.x[p] is undefined and
    they are skipped by the existing truthy guard.
  • Transient group-children (partOfGroup) are invisible (filtered out before drawing, opacity 0)
    and carry no links, so they are skipped explicitly.

Testing

  • npx @biomejs/biome lint src/traces/sankey/render.js passes.
  • I was not able to run the full Jasmine / image suite locally (heavy browser build), so I have
    not run the test suite. The change is a no-op for the existing sankey_x_y and
    sankey_x_y_with_arrows image mocks — all of their nodes are linked and there are no groups, so
    pointNumber === array index for every node and the assigned positions are identical. The other
    sankey mocks don't set both node.x and node.y, so they never enter this branch. I'd expect the
    image baselines to be unchanged, but I'm relying on CI to confirm.
  • Happy to add a regression test if you'd like — e.g. an arrangement: 'fixed' trace with an
    unlinked node placed before a linked one, asserting the linked node lands at its specified
    coordinate.

I'll add the draftlogs/<PR number>_fix.md changelog entry in a follow-up commit per the
contributing guide.

Note

There is a related path I deliberately left out of scope: persistFinalNodePositions (~L799) also
walks graph.nodes positionally to write node.x / node.y back after a drag, so the same
index/order mismatch could affect the saved positions when isolated or grouped nodes are present.
That only surfaces through drag interaction (not the static rendering reported here) and is a more
involved change, so I kept this PR to the rendering fix. Happy to follow up on it separately if
that's useful.

Closes #7758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explicit node coordinates (x/y positions) not fully respected in Sankey diagram

1 participant