diff --git a/draftlogs/7725_fix.md b/draftlogs/7725_fix.md new file mode 100644 index 00000000000..45b670d124e --- /dev/null +++ b/draftlogs/7725_fix.md @@ -0,0 +1 @@ + - Fix sankey nodes being clipped at the bottom edge when using user-positioned nodes (`node.x`/`node.y`) near `y=1.0` or with `arrangement="snap"` collision resolution [[#7725](https://github.com/plotly/plotly.js/pull/7725)] diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index a07d18a9c9e..cee6d7ec362 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -203,6 +203,29 @@ function sankeyModel(layout, d, traceIndex) { }); } + // Push any overlapping nodes up, bounded by the bottom of the plot area. + // Counterpart to resolveCollisionsTopToBottom: that pass only ever moves + // nodes down, so a column clustered near the bottom edge gets walked off + // the plot area. Mirrors the like-named helper in @plotly/d3-sankey, which + // bounds its result on both edges. Nodes that already fit do not move. + function resolveCollisionsBottomToTop(columns) { + columns.forEach(function(nodes) { + var node; + var dy; + var y = height; + var i; + nodes.sort(function(a, b) { + return a.y0 - b.y0; + }); + for(i = nodes.length - 1; i >= 0; --i) { + node = nodes[i]; + dy = node.y1 - y; + if(dy > 1e-6) node.y0 -= dy, node.y1 -= dy; + y = node.y0 - nodePad; + } + }); + } + // Group nodes into columns based on their x position function snapToColumns(nodes) { // Sort nodes by x position @@ -251,14 +274,21 @@ function sankeyModel(layout, d, traceIndex) { 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; + // Keep the node inside the plot area: trace.node.y positions the + // node's centre, so y near 1 would put half the node below the + // bottom edge. When the node is taller than the plot area the + // bounds invert and Lib.constrain pins it to the top edge, which + // beats letting it hang off the top. + var yCenter = Lib.constrain(pos[1], nodeHeight / 2, height - nodeHeight / 2); + graph.nodes[i].y0 = yCenter - nodeHeight / 2; + graph.nodes[i].y1 = yCenter + nodeHeight / 2; } } if(trace.arrangement === 'snap') { nodes = graph.nodes; var columns = snapToColumns(nodes); resolveCollisionsTopToBottom(columns); + resolveCollisionsBottomToTop(columns); } // Update links sankey.update(graph); diff --git a/test/jasmine/tests/sankey_test.js b/test/jasmine/tests/sankey_test.js index 76fa0e9f27c..a75e50e2055 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -578,6 +578,77 @@ describe('sankey tests', function() { .then(done, done.fail); }); + // Measure node rects against the plot area from _fullLayout._size, which + // is what plot.js passes to the renderer as its `height`. Do NOT measure + // against the '.sankey' layer's bounding box: node rects are descendants + // of it, and an SVG group's box is the union of its children, so it grows + // to contain any overflow and the assertion can never fail. + function expectNodesWithinPlotArea(gd, msg) { + var gs = gd._fullLayout._size; + var gdTop = gd.getBoundingClientRect().top; + var plotTop = gdTop + gs.t; + var plotBottom = plotTop + gs.h; + var nodeRects = gd.querySelectorAll('.sankey-node .node-rect'); + + expect(nodeRects.length).toBeGreaterThan(0, msg + ': found no node rects'); + + for(var i = 0; i < nodeRects.length; i++) { + var rect = nodeRects[i].getBoundingClientRect(); + expect(rect.bottom).toBeLessThan(plotBottom + 1, + msg + ': node ' + i + ' extends ' + (rect.bottom - plotBottom).toFixed(1) + + 'px past the bottom of the plot area'); + expect(rect.top).toBeGreaterThan(plotTop - 1, + msg + ': node ' + i + ' extends ' + (plotTop - rect.top).toFixed(1) + + 'px past the top of the plot area'); + } + } + + it('keeps an explicitly positioned node inside the plot area', function(done) { + // arrangement 'fixed' skips collision resolution, so this covers the + // node.y centring path on its own. Node B is requested at y = 0.98; + // unclamped it overflows by nodeHeight/2 - (1 - 0.98) * height. + Plotly.newPlot(gd, [{ + type: 'sankey', + arrangement: 'fixed', + node: { + label: ['A', 'B', 'C'], + x: [0.1, 0.1, 0.9], + y: [0.3, 0.98, 0.5], + pad: 10 + }, + link: {source: [0, 1], target: [2, 2], value: [10, 10]} + }], {width: 600, height: 300, margin: {l: 10, r: 10, t: 10, b: 10}}) + .then(function() { + expectNodesWithinPlotArea(gd, 'fixed arrangement'); + }) + .then(done, done.fail); + }); + + it('keeps a snapped column inside the plot area when collisions cascade', function(done) { + // Every y is <= 0.92, so the centring clamp alone would not prevent + // clipping here: B, C and D share a column and overlap, and resolving + // those collisions downward walks the column off the bottom edge. + Plotly.newPlot(gd, [{ + type: 'sankey', + arrangement: 'snap', + node: { + label: ['A', 'B', 'C', 'D', 'E'], + x: [0.1, 0.5, 0.5, 0.5, 0.9], + y: [0.5, 0.80, 0.86, 0.92, 0.5], + pad: 10 + }, + link: { + source: [0, 0, 0, 1, 2, 3], + target: [1, 2, 3, 4, 4, 4], + value: [8, 8, 8, 8, 8, 8] + } + }], {width: 600, height: 400, margin: {l: 10, r: 10, t: 10, b: 10}}) + .then(function() { + expectNodesWithinPlotArea(gd, 'snap arrangement'); + }) + .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'));