diff --git a/draftlogs/7986_fix.md b/draftlogs/7986_fix.md new file mode 100644 index 00000000000..482e6eb38f0 --- /dev/null +++ b/draftlogs/7986_fix.md @@ -0,0 +1 @@ +- Fix the `node.pad` clamp warning in Sankey traces: it never fired after the `@plotly/d3-sankey` v0.12 upgrade because the layout getter now returns the configured padding rather than the effective (post-clamp) value; the warning now measures the laid-out node gaps and fires with the effective padding [[#7986](https://github.com/plotly/plotly.js/pull/7986)] diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index 83f8bbb2f98..093317e0bdc 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -90,8 +90,41 @@ function sankeyModel(layout, d, traceIndex) { var graph = sankey(); - if(sankey.nodePadding() < nodePad) { - Lib.warn('node.pad was reduced to ', sankey.nodePadding(), ' to fit within the figure.'); + /* + * Detect a clamped node.pad from the laid-out node positions rather than + * from `sankey.nodePadding()`: since @plotly/d3-sankey v0.12 the getter + * returns the *configured* value (the post-clamp padding is kept in an + * internal variable), so comparing against it never fires. Measuring the + * smallest gap between consecutive nodes sharing a column works for both + * @plotly/d3-sankey and @plotly/d3-sankey-circular. + */ + var effectiveNodePad = null; + if(graph.nodes.length > 1) { + var colAttr = horizontal ? 'x0' : 'y0'; + var posAttr = horizontal ? 'y0' : 'x0'; + var columns = {}; + graph.nodes.forEach(function(n) { + var ck = n[colAttr]; + if(!columns[ck]) columns[ck] = []; + columns[ck].push(n); + }); + Object.keys(columns).forEach(function(ck) { + var col = columns[ck].sort(function(a, b) { return a[posAttr] - b[posAttr]; }); + for(var i = 1; i < col.length; i++) { + var gap = col[i][posAttr] - col[i - 1][posAttr]; + if(effectiveNodePad === null || gap < effectiveNodePad) { + effectiveNodePad = gap; + } + } + }); + } + + if(effectiveNodePad !== null && effectiveNodePad < nodePad) { + Lib.warn( + 'node.pad was reduced to ', + Math.round(effectiveNodePad * 100) / 100, + ' to fit within the figure.' + ); } // Counters for nested loops diff --git a/test/jasmine/tests/sankey_test.js b/test/jasmine/tests/sankey_test.js index d82ecdf9296..dbce9eb7422 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -389,6 +389,71 @@ describe('sankey tests', function () { }); }); + describe('node.pad clamp warning (issue 7832)', function() { + var gd; + beforeEach(function() { + gd = createGraphDiv(); + }); + afterEach(destroyGraphDiv); + + function padWarnings() { + var warnings = []; + spyOn(Lib, 'warn').and.callFake(function(msg) { + warnings.push(msg); + }); + return warnings; + } + + it('fires when node.pad is clamped to fit the figure', function(done) { + var warnings = padWarnings(); + var labels = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i']; + // 8 sibling nodes cannot fit in a ~300px-high domain with + // node.pad: 40 -> d3-sankey clamps the padding + Plotly.newPlot(gd, [{ + type: 'sankey', + domain: { x: [0, 1], y: [0, 0.5] }, + node: { + label: labels, + pad: 40, + thickness: 15 + }, + link: { + source: [0, 0, 0, 0, 0, 0, 0, 0], + target: [1, 2, 3, 4, 5, 6, 7, 8], + value: [1, 1, 1, 1, 1, 1, 1, 1] + } + }]) + .then(function() { + expect(warnings.length).toBe(1); + expect(warnings[0]).toContain('node.pad was reduced'); + }) + .then(done, done.fail); + }); + + it('does not fire when node.pad fits', function(done) { + var warnings = padWarnings(); + + Plotly.newPlot(gd, [{ + type: 'sankey', + domain: { x: [0, 1], y: [0, 1] }, + node: { + label: ['a', 'b', 'c', 'd'], + pad: 20, + thickness: 10 + }, + link: { + source: [0, 0, 0], + target: [1, 2, 3], + value: [1, 1, 1] + } + }]) + .then(function() { + expect(warnings.length).toBe(0); + }) + .then(done, done.fail); + }); + }); + describe('lifecycle methods', function () { var gd; beforeEach(function () {