diff --git a/draftlogs/7977_fix.md b/draftlogs/7977_fix.md new file mode 100644 index 00000000000..56a42eef789 --- /dev/null +++ b/draftlogs/7977_fix.md @@ -0,0 +1 @@ + - Fix `node.pad` reduction warning so it derives the effective (post-clamp) padding from the laid-out node geometry instead of reading `sankey.nodePadding()`, which since @plotly/d3-sankey 0.12.x returns the configured value and made the warning never fire [[#7977](https://github.com/plotly/plotly.js/pull/7977)] diff --git a/src/traces/sankey/render.js b/src/traces/sankey/render.js index 83f8bbb2f98..87116e31d99 100644 --- a/src/traces/sankey/render.js +++ b/src/traces/sankey/render.js @@ -90,8 +90,30 @@ 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.'); + // Derive the effective (post-clamp) node padding from the laid-out node + // geometry instead of reading it back through `sankey.nodePadding()`. + // In @plotly/d3-sankey@0.7.x that getter returned the clamped value after + // the layout ran, but since 0.12.x it returns the user-configured value + // (upstream split `dy` from `py`), so a getter-based check would never + // fire. Measuring the smallest vertical gap between consecutive nodes in + // any one column is version-independent. See #7832. + var effectivePad = nodePad; + var columns = {}; + graph.nodes.forEach(function(node) { + var col = Math.round(node.x0); + if(!columns[col]) columns[col] = []; + columns[col].push([node.y0, node.y1]); + }); + for(var key in columns) { + var column = columns[key].sort(function(a, b) { return a[0] - b[0]; }); + for(var n = 1; n < column.length; n++) { + var gap = column[n][0] - column[n - 1][1]; + if(gap < effectivePad) effectivePad = gap; + } + } + + if(effectivePad < nodePad) { + Lib.warn('node.pad was reduced to ', effectivePad, ' 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..6feb2eae349 100644 --- a/test/jasmine/tests/sankey_test.js +++ b/test/jasmine/tests/sankey_test.js @@ -95,8 +95,88 @@ describe('sankey tests', function () { }); }); + describe('sankey global defaults', function () { it('should not coerce trace opacity', function () { + + + describe('node.pad reduction warning', function() { + // The warning must be driven by the effective (post-clamp) padding, + // not by reading `sankey.nodePadding()` back, which since + // @plotly/d3-sankey@0.12.x returns the configured value instead of + // the clamped one - see #7832. + var padMock = { + data: [{ + type: 'sankey', + node: { + label: Array.from({length: 24}, function(_, i) { return 'n' + i; }), + pad: 30, + thickness: 10 + }, + link: { + // star topology: one source feeding 24 sinks puts all 24 + // sink nodes in a single column, so a small figure must + // clamp the padding + source: Array.from({length: 24}, function() { return 0; }), + target: Array.from({length: 24}, function(_, i) { return i + 1; }), + value: Array.from({length: 24}, function() { return 1; }) + } + }], + layout: { + width: 500, + height: 500, + margin: {l: 10, r: 10, t: 10, b: 10} + } + }; + + it('warns when the figure is too small for node.pad', function(done) { + var warnings = []; + spyOn(Lib, 'warn').and.callFake(function() { + // collect all arguments, as Lib.warn is variadic + warnings.push(Array.prototype.slice.call(arguments)); + }); + + var fig = Lib.extendDeep({}, padMock); + fig.layout.width = 200; + fig.layout.height = 100; + var gd = createGraphDiv(); + Plotly.newPlot(gd, fig) + .then(function() { + expect(warnings.length).toEqual(1); + expect(warnings[0][0]).toBe('node.pad was reduced to '); + expect(warnings[0][1]).toBeLessThan(30); + return Plotly.purge(gd); + }) + .then(function() { destroyGraphDiv(gd); }) + .then(done, done.fail); + }); + + it('does not warn when the figure fits node.pad', function(done) { + var warnings = []; + spyOn(Lib, 'warn').and.callFake(function() { + // collect all arguments, as Lib.warn is variadic + warnings.push(Array.prototype.slice.call(arguments)); + }); + + var fig = Lib.extendDeep({}, padMock); + fig.layout.width = 480; + fig.layout.height = 1000; + // keep the sink column short enough that pad=30 always fits + fig.data[0].node.label = Array.from({length: 8}, function(_, i) { return 'n' + i; }); + fig.data[0].link.source = Array.from({length: 7}, function() { return 0; }); + fig.data[0].link.target = Array.from({length: 7}, function(_, i) { return i + 1; }); + fig.data[0].link.value = Array.from({length: 7}, function() { return 1; }); + var gd = createGraphDiv(); + Plotly.newPlot(gd, fig) + .then(function() { + expect(warnings.length).toEqual(0); + return Plotly.purge(gd); + }) + .then(function() { destroyGraphDiv(gd); }) + .then(done, done.fail); + }); + }); + (fix(sankey): derive effective node.pad clamp from layout geometry) var gd = Lib.extendDeep({}, mock); supplyAllDefaults(gd);