Skip to content
Open
1 change: 1 addition & 0 deletions draftlogs/7977_fix.md
Original file line number Diff line number Diff line change
@@ -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)]
26 changes: 24 additions & 2 deletions src/traces/sankey/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
80 changes: 80 additions & 0 deletions test/jasmine/tests/sankey_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading