diff --git a/draftlogs/7826_fix.md b/draftlogs/7826_fix.md new file mode 100644 index 00000000000..6663d8d09f3 --- /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 [[#7826](https://github.com/plotly/plotly.js/pull/7826)], with thanks to @kyo5uke for the contribution! diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index 83f8bbb2f98..30a7df024d1 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -261,15 +261,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') { @@ -825,13 +833,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], @@ -1110,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 d82ecdf9296..9220f95d7c3 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -696,6 +696,101 @@ 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) { + // measure the rect because the label shifts the group bounding box + centers[d.index] = getNodeCoords(this.querySelector('.node-rect')); + }); + 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('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'));