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

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!

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);
-};
+};

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'));