Skip to content

Commit 81bb1cd

Browse files
committed
fix(sankey): restore node.pad clamp warning broken by d3-sankey v0.12
The warning compared the layout getter sankey.nodePadding() against the configured node.pad. Since @plotly/d3-sankey 0.12.3 (#7830) that getter returns the *configured* value (the post-clamp padding lives in an internal variable), so the comparison never fired and users no longer learn their pad was reduced. Measure the smallest gap between consecutive nodes sharing a column instead: it reflects the effective padding and works for both @plotly/d3-sankey and @plotly/d3-sankey-circular. Fixes #7832
1 parent 8bc6ee5 commit 81bb1cd

3 files changed

Lines changed: 101 additions & 2 deletions

File tree

draftlogs/7986_fix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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)]

src/traces/sankey/render.js

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,41 @@ function sankeyModel(layout, d, traceIndex) {
9090

9191
var graph = sankey();
9292

93-
if(sankey.nodePadding() < nodePad) {
94-
Lib.warn('node.pad was reduced to ', sankey.nodePadding(), ' to fit within the figure.');
93+
/*
94+
* Detect a clamped node.pad from the laid-out node positions rather than
95+
* from `sankey.nodePadding()`: since @plotly/d3-sankey v0.12 the getter
96+
* returns the *configured* value (the post-clamp padding is kept in an
97+
* internal variable), so comparing against it never fires. Measuring the
98+
* smallest gap between consecutive nodes sharing a column works for both
99+
* @plotly/d3-sankey and @plotly/d3-sankey-circular.
100+
*/
101+
var effectiveNodePad = null;
102+
if(graph.nodes.length > 1) {
103+
var colAttr = horizontal ? 'x0' : 'y0';
104+
var posAttr = horizontal ? 'y0' : 'x0';
105+
var columns = {};
106+
graph.nodes.forEach(function(n) {
107+
var ck = n[colAttr];
108+
if(!columns[ck]) columns[ck] = [];
109+
columns[ck].push(n);
110+
});
111+
Object.keys(columns).forEach(function(ck) {
112+
var col = columns[ck].sort(function(a, b) { return a[posAttr] - b[posAttr]; });
113+
for(var i = 1; i < col.length; i++) {
114+
var gap = col[i][posAttr] - col[i - 1][posAttr];
115+
if(effectiveNodePad === null || gap < effectiveNodePad) {
116+
effectiveNodePad = gap;
117+
}
118+
}
119+
});
120+
}
121+
122+
if(effectiveNodePad !== null && effectiveNodePad < nodePad) {
123+
Lib.warn(
124+
'node.pad was reduced to ',
125+
Math.round(effectiveNodePad * 100) / 100,
126+
' to fit within the figure.'
127+
);
95128
}
96129

97130
// Counters for nested loops

test/jasmine/tests/sankey_test.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,71 @@ describe('sankey tests', function () {
389389
});
390390
});
391391

392+
describe('node.pad clamp warning (issue 7832)', function() {
393+
var gd;
394+
beforeEach(function() {
395+
gd = createGraphDiv();
396+
});
397+
afterEach(destroyGraphDiv);
398+
399+
function padWarnings() {
400+
var warnings = [];
401+
spyOn(Lib, 'warn').and.callFake(function(msg) {
402+
warnings.push(msg);
403+
});
404+
return warnings;
405+
}
406+
407+
it('fires when node.pad is clamped to fit the figure', function(done) {
408+
var warnings = padWarnings();
409+
var labels = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i'];
410+
// 8 sibling nodes cannot fit in a ~300px-high domain with
411+
// node.pad: 40 -> d3-sankey clamps the padding
412+
Plotly.newPlot(gd, [{
413+
type: 'sankey',
414+
domain: { x: [0, 1], y: [0, 0.5] },
415+
node: {
416+
label: labels,
417+
pad: 40,
418+
thickness: 15
419+
},
420+
link: {
421+
source: [0, 0, 0, 0, 0, 0, 0, 0],
422+
target: [1, 2, 3, 4, 5, 6, 7, 8],
423+
value: [1, 1, 1, 1, 1, 1, 1, 1]
424+
}
425+
}])
426+
.then(function() {
427+
expect(warnings.length).toBe(1);
428+
expect(warnings[0]).toContain('node.pad was reduced');
429+
})
430+
.then(done, done.fail);
431+
});
432+
433+
it('does not fire when node.pad fits', function(done) {
434+
var warnings = padWarnings();
435+
436+
Plotly.newPlot(gd, [{
437+
type: 'sankey',
438+
domain: { x: [0, 1], y: [0, 1] },
439+
node: {
440+
label: ['a', 'b', 'c', 'd'],
441+
pad: 20,
442+
thickness: 10
443+
},
444+
link: {
445+
source: [0, 0, 0],
446+
target: [1, 2, 3],
447+
value: [1, 1, 1]
448+
}
449+
}])
450+
.then(function() {
451+
expect(warnings.length).toBe(0);
452+
})
453+
.then(done, done.fail);
454+
});
455+
});
456+
392457
describe('lifecycle methods', function () {
393458
var gd;
394459
beforeEach(function () {

0 commit comments

Comments
 (0)