From bf955679d25e96abd9cb10252208de1d989c3b41 Mon Sep 17 00:00:00 2001 From: kyosuke Date: Sat, 6 Jun 2026 05:54:15 +0900 Subject: [PATCH 1/7] Fix explicit node positions in fixed-arrangement Sankey --- src/traces/sankey/render.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index a07d18a9c9e..b528a5f5b96 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -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. + 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]) { + 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') { From 86084f9a39bc2e04649fcac4a702db75a29c4c5c Mon Sep 17 00:00:00 2001 From: kyosuke Date: Sat, 6 Jun 2026 05:56:54 +0900 Subject: [PATCH 2/7] Add draftlog for #7826 --- draftlogs/7826_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/7826_fix.md diff --git a/draftlogs/7826_fix.md b/draftlogs/7826_fix.md new file mode 100644 index 00000000000..4290a25a1ef --- /dev/null +++ b/draftlogs/7826_fix.md @@ -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)] From c07bbf4c8425924f4b10290932712cad4f1d0bae Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 27 Oct 2026 11:31:16 -0600 Subject: [PATCH 3/7] Fix explicit Sankey node positions with unlinked or grouped nodes --- src/traces/sankey/render.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index b528a5f5b96..1e8d7f481d3 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -253,7 +253,8 @@ function sankeyModel(layout, d, traceIndex) { if(forcedNode.partOfGroup) continue; var p = forcedNode.pointNumber; - if(trace.node.x[p] && trace.node.y[p]) { + // Allow values of zero for coordinates + 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; @@ -805,13 +806,13 @@ function snappingForce(sankeyNode, forceKey, nodes, d) { // basic data utilities function persistFinalNodePositions(d, gd) { - var x = []; - var y = []; - for(var i = 0; i < d.graph.nodes.length; i++) { - var nodeX = (d.graph.nodes[i].x0 + d.graph.nodes[i].x1) / 2; - var nodeY = (d.graph.nodes[i].y0 + d.graph.nodes[i].y1) / 2; - x.push(nodeX / d.figure.width); - y.push(nodeY / d.figure.height); + // Write by pointNumber because graph.nodes omits unlinked nodes + const x = [...d.trace.node.x]; + const y = [...d.trace.node.y]; + for (const node of d.graph.nodes) { + if (node.partOfGroup) continue; + x[node.pointNumber] = (node.x0 + node.x1) / 2 / d.figure.width; + y[node.pointNumber] = (node.y0 + node.y1) / 2 / d.figure.height; } Registry.call('_guiRestyle', gd, { 'node.x': [x], From 0c7b1eac87d100b0bf3258c6f68941b6c268d4b3 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 27 Oct 2026 11:31:23 -0600 Subject: [PATCH 4/7] Add tests --- test/jasmine/tests/sankey_test.js | 106 ++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/test/jasmine/tests/sankey_test.js b/test/jasmine/tests/sankey_test.js index 69120330795..8d84d64ef3d 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -578,6 +578,112 @@ describe('sankey tests', function() { .then(done, done.fail); }); + describe('explicit node.(x|y) positions', () => { + // Node 1 has no links, so it is absent from the rendered graph + const makeFixedMock = () => ({ + data: [ + { + type: 'sankey', + arrangement: 'fixed', + node: { + label: ['a', 'unlinked', 'b', 'c', 'd'], + x: [0.1, 0.5, 0.5, 0.9, 0.9], + y: [0.2, 0.5, 0.3, 0.8, 0.1], + pad: 5 + }, + link: { + source: [0, 0, 2], + target: [2, 3, 4], + value: [1, 1, 1] + } + } + ], + layout: { width: 600, height: 400 } + }); + + const getCentersByIndex = () => { + const centers = {}; + // d3 v3 binds the element to `this`, so this callback stays a function + d3SelectAll('.sankey-node').each(function (d) { + centers[d.index] = getNodeCoords(this); + }); + return centers; + }; + + const assertPositions = (trace, centers, indices, msg) => { + const size = gd._fullLayout._size; + const ref = indices[0]; + indices.forEach((i) => { + const dx = (trace.node.x[i] - trace.node.x[ref]) * size.w; + const dy = (trace.node.y[i] - trace.node.y[ref]) * size.h; + expect(centers[i].x - centers[ref].x).toBeCloseTo(dx, 0, msg + ': node ' + i + ' x'); + expect(centers[i].y - centers[ref].y).toBeCloseTo(dy, 0, msg + ': node ' + i + ' y'); + }); + }; + + it('applies positions to the right nodes when a node is unlinked', (done) => { + const fig = makeFixedMock(); + + Plotly.newPlot(gd, fig) + .then(() => { + assertPositions(fig.data[0], getCentersByIndex(), [0, 2, 3, 4], 'fixed'); + }) + .then(done, done.fail); + }); + + it('respects explicit positions of 0', (done) => { + const fig = makeFixedMock(); + fig.data[0].node.x[2] = 0; + fig.data[0].node.y[3] = 0; + + Plotly.newPlot(gd, fig) + .then(() => { + assertPositions(fig.data[0], getCentersByIndex(), [0, 2, 3, 4], 'zero'); + }) + .then(done, done.fail); + }); + + it('applies positions to the right nodes when nodes are grouped', (done) => { + const fig = makeFixedMock(); + // Grouping nodes 3 and 4 leaves them unlinked and adds group node 5 + fig.data[0].node.groups = [[3, 4]]; + + Plotly.newPlot(gd, fig) + .then(() => { + assertPositions(fig.data[0], getCentersByIndex(), [0, 2], 'grouped'); + }) + .then(done, done.fail); + }); + + it('keeps the positions of the other nodes after a drag when a node is unlinked', (done) => { + const fig = makeFixedMock(); + fig.data[0].arrangement = 'freeform'; + let before; + + Plotly.newPlot(gd, fig) + .then(() => { + before = getCentersByIndex(); + const node = d3SelectAll('.sankey-node') + .filter((d) => d.index === 2) + .node(); + return drag({ node: node, dpos: [0, 30] }); + }) + .then(() => { + const { x } = gd._fullData[0].node; + expect(x.length).toBe(5, 'x covers the unlinked node'); + expect(x[1]).toBeCloseTo(0.5, 5, 'unlinked node x is kept'); + + const after = getCentersByIndex(); + [0, 3, 4].forEach((i) => { + expect(after[i].x).toBeCloseTo(before[i].x, 0, 'node ' + i + ' x'); + expect(after[i].y).toBeCloseTo(before[i].y, 0, 'node ' + i + ' y'); + }); + expect(after[2].y).toBeCloseTo(before[2].y + 30, 0, 'dragged node y'); + }) + .then(done, done.fail); + }); + }); + it('resets each subplot to its initial view (ie. x, y groups) via modebar button', function(done) { var mockCopy = Lib.extendDeep({}, require('../../image/mocks/sankey_subplots_circular')); From 6dbfa2d7c50b9b6378b5a8e490ed4829afa98d90 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 27 Oct 2026 11:33:08 -0600 Subject: [PATCH 5/7] Update draftlog --- draftlogs/7826_fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draftlogs/7826_fix.md b/draftlogs/7826_fix.md index 4290a25a1ef..6663d8d09f3 100644 --- a/draftlogs/7826_fix.md +++ b/draftlogs/7826_fix.md @@ -1 +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)] +- Respect explicit `node.x`/`node.y` positions in Sankey traces that contain unlinked or grouped nodes [[#7826](https://github.com/plotly/plotly.js/pull/7826)], with thanks to @kyo5uke for the contribution! From dc26aa9e2fce5695f5056185f0ef4b6847bf8b00 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 27 Oct 2026 15:01:27 -0600 Subject: [PATCH 6/7] Revert handling of zero coords --- src/traces/sankey/render.js | 5 ++--- test/jasmine/tests/sankey_test.js | 12 ------------ 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index 4f47d8ff301..30a7df024d1 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -270,8 +270,7 @@ function sankeyModel(layout, d, traceIndex) { if(forcedNode.partOfGroup) continue; var p = forcedNode.pointNumber; - // Allow values of zero for coordinates - if (trace.node.x[p] != null && trace.node.y[p] != null) { + if (trace.node.x[p] && trace.node.y[p]) { var pos = [trace.node.x[p] * width, trace.node.y[p] * height]; forcedNode.x0 = pos[0] - nodeThickness / 2; forcedNode.x1 = pos[0] + nodeThickness / 2; @@ -1119,4 +1118,4 @@ module.exports = function(gd, svg, calcData, layout, callbacks) { nodeLabel .transition() .ease(c.ease).duration(c.duration); -}; \ No newline at end of file +}; diff --git a/test/jasmine/tests/sankey_test.js b/test/jasmine/tests/sankey_test.js index 36c3d99b8fa..a9044f64fcd 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -749,18 +749,6 @@ describe('sankey tests', function () { .then(done, done.fail); }); - it('respects explicit positions of 0', (done) => { - const fig = makeFixedMock(); - fig.data[0].node.x[2] = 0; - fig.data[0].node.y[3] = 0; - - Plotly.newPlot(gd, fig) - .then(() => { - assertPositions(fig.data[0], getCentersByIndex(), [0, 2, 3, 4], 'zero'); - }) - .then(done, done.fail); - }); - it('applies positions to the right nodes when nodes are grouped', (done) => { const fig = makeFixedMock(); // Grouping nodes 3 and 4 leaves them unlinked and adds group node 5 From e6ea65d5e6c9c1cf1ddf1451a86538715d41d64d Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 27 Oct 2026 15:44:14 -0600 Subject: [PATCH 7/7] Fix test --- test/jasmine/tests/sankey_test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/sankey_test.js b/test/jasmine/tests/sankey_test.js index a9044f64fcd..9220f95d7c3 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -723,7 +723,8 @@ describe('sankey tests', function () { const centers = {}; // d3 v3 binds the element to `this`, so this callback stays a function d3SelectAll('.sankey-node').each(function (d) { - centers[d.index] = getNodeCoords(this); + // measure the rect because the label shifts the group bounding box + centers[d.index] = getNodeCoords(this.querySelector('.node-rect')); }); return centers; };