fix(sankey): derive effective node.pad clamp from layout geometry

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)]

src/traces/sankey/render.js

@@ -90,8 +90,32 @@ 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;
+        }
+    }
+
+    // Allow for floating-point rounding when the effective gap is exactly
+    // nodePad (for example, 29.999999999999943 for a requested pad of 30).
+    if(effectivePad < nodePad - 1e-6) {
+        Lib.warn('node.pad was reduced to ', effectivePad, ' to fit within the figure.');
     }
 
     // Counters for nested loops
@@ -1110,4 +1134,4 @@ module.exports = function(gd, svg, calcData, layout, callbacks) {
     nodeLabel
         .transition()
         .ease(c.ease).duration(c.duration);
-};
+};

test/jasmine/tests/sankey_test.js

@@ -95,6 +95,7 @@ describe('sankey tests', function () {
         });
     });
 
+
     describe('sankey global defaults', function () {
         it('should not coerce trace opacity', function () {
             var gd = Lib.extendDeep({}, mock);
@@ -103,6 +104,83 @@ describe('sankey tests', function () {
 
             expect(gd._fullData[0].opacity).toBeUndefined();
         });
+
+        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: 25}, 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);
+            });
+        });
     });
 
     describe('sankey defaults', function () {