Merge pull request #5315 from plotly/fix5314-reduce-minfinalheight

archmoj · web-flow · commit 6cf9f3213a9c · 2020-12-04T11:20:25.000-05:00
Adjust minfinalheight in automargin routine to allow narrow subplots e.g. gauge
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -1866,8 +1866,13 @@ function initMargins(fullLayout) {
     if(!fullLayout._pushmarginIds) fullLayout._pushmarginIds = {};
 }
 
-var minFinalWidth = 64; // could possibly be exposed as layout.margin.minfinalwidth
-var minFinalHeight = 64; // could possibly be exposed as layout.margin.minfinalheight
+// non-negotiable - this is the smallest height we will allow users to specify via explicit margins
+var MIN_SPECIFIED_WIDTH = 2;
+var MIN_SPECIFIED_HEIGHT = 2;
+
+// could be exposed as an option - the smallest we will allow automargin to shrink a larger plot
+var MIN_REDUCED_WIDTH = 64;
+var MIN_REDUCED_HEIGHT = 64;
 
 /**
  * autoMargin: called by components that may need to expand the margins to
@@ -1888,36 +1893,53 @@ plots.autoMargin = function(gd, id, o) {
     var fullLayout = gd._fullLayout;
     var width = fullLayout.width;
     var height = fullLayout.height;
+    var margin = fullLayout.margin;
+
+    var minFinalWidth = Lib.constrain(
+        width - margin.l - margin.r,
+        MIN_SPECIFIED_WIDTH,
+        MIN_REDUCED_WIDTH
+    );
+
+    var minFinalHeight = Lib.constrain(
+        height - margin.t - margin.b,
+        MIN_SPECIFIED_HEIGHT,
+        MIN_REDUCED_HEIGHT
+    );
+
     var maxSpaceW = Math.max(0, width - minFinalWidth);
     var maxSpaceH = Math.max(0, height - minFinalHeight);
 
     var pushMargin = fullLayout._pushmargin;
     var pushMarginIds = fullLayout._pushmarginIds;
 
-    if(fullLayout.margin.autoexpand !== false) {
+    if(margin.autoexpand !== false) {
         if(!o) {
             delete pushMargin[id];
             delete pushMarginIds[id];
         } else {
             var pad = o.pad;
             if(pad === undefined) {
-                var margin = fullLayout.margin;
                 // if no explicit pad is given, use 12px unless there's a
                 // specified margin that's smaller than that
                 pad = Math.min(12, margin.l, margin.r, margin.t, margin.b);
             }
 
             // if the item is too big, just give it enough automargin to
             // make sure you can still grab it and bring it back
-            var rW = (o.l + o.r) / maxSpaceW;
-            if(rW > 1) {
-                o.l /= rW;
-                o.r /= rW;
+            if(maxSpaceW) {
+                var rW = (o.l + o.r) / maxSpaceW;
+                if(rW > 1) {
+                    o.l /= rW;
+                    o.r /= rW;
+                }
             }
-            var rH = (o.t + o.b) / maxSpaceH;
-            if(rH > 1) {
-                o.t /= rH;
-                o.b /= rH;
+            if(maxSpaceH) {
+                var rH = (o.t + o.b) / maxSpaceH;
+                if(rH > 1) {
+                    o.t /= rH;
+                    o.b /= rH;
+                }
             }
 
             var xl = o.xl !== undefined ? o.xl : o.x;
@@ -1944,8 +1966,6 @@ plots.doAutoMargin = function(gd) {
     var fullLayout = gd._fullLayout;
     var width = fullLayout.width;
     var height = fullLayout.height;
-    var maxSpaceW = Math.max(0, width - minFinalWidth);
-    var maxSpaceH = Math.max(0, height - minFinalHeight);
 
     if(!fullLayout._size) fullLayout._size = {};
     initMargins(fullLayout);
@@ -2018,16 +2038,35 @@ plots.doAutoMargin = function(gd) {
         }
     }
 
-    var rW = (ml + mr) / maxSpaceW;
-    if(rW > 1) {
-        ml /= rW;
-        mr /= rW;
+    var minFinalWidth = Lib.constrain(
+        width - margin.l - margin.r,
+        MIN_SPECIFIED_WIDTH,
+        MIN_REDUCED_WIDTH
+    );
+
+    var minFinalHeight = Lib.constrain(
+        height - margin.t - margin.b,
+        MIN_SPECIFIED_HEIGHT,
+        MIN_REDUCED_HEIGHT
+    );
+
+    var maxSpaceW = Math.max(0, width - minFinalWidth);
+    var maxSpaceH = Math.max(0, height - minFinalHeight);
+
+    if(maxSpaceW) {
+        var rW = (ml + mr) / maxSpaceW;
+        if(rW > 1) {
+            ml /= rW;
+            mr /= rW;
+        }
     }
 
-    var rH = (mb + mt) / maxSpaceH;
-    if(rH > 1) {
-        mb /= rH;
-        mt /= rH;
+    if(maxSpaceH) {
+        var rH = (mb + mt) / maxSpaceH;
+        if(rH > 1) {
+            mb /= rH;
+            mt /= rH;
+        }
     }
 
     gs.l = Math.round(ml);
diff --git a/test/image/baselines/automargin-narrow-indicator.png b/test/image/baselines/automargin-narrow-indicator.png
diff --git a/test/image/mocks/automargin-narrow-indicator.json b/test/image/mocks/automargin-narrow-indicator.json
@@ -0,0 +1,50 @@
+{
+    "config": {
+        "displayModeBar": false
+    },
+    "data": [
+        {
+            "type": "indicator",
+            "mode": "number+gauge",
+            "gauge": {
+                "shape": "bullet",
+                "axis": {
+                    "range": [
+                        0,
+                        1
+                    ],
+                    "tickfont": {
+                        "size": 12
+                    }
+                }
+            },
+            "value": 0.5,
+            "number": {
+                "font": {
+                    "size": 48
+                }
+            }
+        }
+    ],
+    "layout": {
+        "width": 300,
+        "height": 60,
+        "margin": {
+            "l": 10,
+            "r": 10,
+            "t": 25,
+            "b": 25
+        },
+        "title": {
+            "text": "Automargin indicator",
+            "xanchor": "left",
+            "x": 0,
+            "yanchor": "top",
+            "y": 1,
+            "pad": {
+                "t": 5,
+                "l": 10
+            }
+        }
+    }
+}
diff --git a/test/jasmine/tests/indicator_test.js b/test/jasmine/tests/indicator_test.js
@@ -170,7 +170,7 @@ describe('Indicator plot', function() {
                 return Plotly.relayout(gd, {width: 200, height: 200});
             })
             .then(function() {
-                checkNumbersScale(0.4794007490636704, 'should scale down');
+                checkNumbersScale(0.2, 'should scale down');
                 return Plotly.relayout(gd, {width: 400, height: 400});
             })
             .then(function() {
diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js
@@ -403,7 +403,7 @@ describe('The legend', function() {
                 expect(countLegendClipPaths(gd)).toBe(1);
 
                 // clippath resized to new height less than new plot height
-                expect(+legendHeight).toBeGreaterThan(getPlotHeight(gd));
+                expect(+legendHeight).toBe(getPlotHeight(gd));
                 expect(+legendHeight).toBeLessThan(+origLegendHeight);
 
                 done();
diff --git a/test/jasmine/tests/mock_test.js b/test/jasmine/tests/mock_test.js
@@ -42,6 +42,7 @@ var list = [
     'automargin-large-margins',
     'automargin-large-margins-both-sides',
     'automargin-large-margins-horizontal',
+    'automargin-narrow-indicator',
     'automargin-mirror-all',
     'automargin-mirror-allticks',
     'automargin-multiline-titles',
@@ -1126,6 +1127,7 @@ figs['arrow-markers'] = require('@mocks/arrow-markers');
 figs['automargin-large-margins'] = require('@mocks/automargin-large-margins');
 figs['automargin-large-margins-both-sides'] = require('@mocks/automargin-large-margins-both-sides');
 figs['automargin-large-margins-horizontal'] = require('@mocks/automargin-large-margins-horizontal');
+figs['automargin-narrow-indicator'] = require('@mocks/automargin-narrow-indicator');
 figs['automargin-mirror-all'] = require('@mocks/automargin-mirror-all');
 figs['automargin-mirror-allticks'] = require('@mocks/automargin-mirror-allticks');
 figs['automargin-multiline-titles'] = require('@mocks/automargin-multiline-titles');
