Apply review comments

hannahker · hannahker · commit 33bbbef70df2 · 2023-03-08T17:06:26.000-08:00
diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js
@@ -277,7 +277,7 @@ function _doPlot(gd, data, layout, config) {
 
         subroutines.drawMarginPushers(gd);
         Axes.allowAutoMargin(gd);
-        Plots.allowAutoMargin(gd, 'title.automargin');
+        if(gd._fullLayout.title.text && gd._fullLayout.title.automargin) Plots.allowAutoMargin(gd, 'title.automargin');
 
         // TODO can this be moved elsewhere?
         if(fullLayout._has('pie')) {
diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js
@@ -406,7 +406,7 @@ exports.drawMainTitle = function(gd) {
     var dy = getMainTitleDy(fullLayout);
     var y = getMainTitleY(fullLayout, dy);
 
-    if(title.automargin) {
+    if(title.text && title.automargin) {
         applyTitleAutoMargin(gd, y);
     }
 
@@ -442,23 +442,15 @@ function isOutsideContainer(gd, title, position, y) {
 // 'auto' is not supported for either title.y or title.yanchor when automargin=true
 function setDflts(title) {
     if(title.automargin && title.yref === 'paper') {
-        title.y = title.y === 0 ? title.y : 1;
+        title.y = title.y === 0 ? 0 : 1;
         if(title.yanchor === 'auto') {
-            if(title.y === 0) {
-                title.yanchor = 'top';
-            } else {
-                title.yanchor = 'bottom';
-            }
+            title.yanchor = title.y === 0 ? 'top' : 'bottom';
         }
     }
     if(title.automargin && title.yref === 'container') {
-        title.y = title.y === 'auto' ? 1 : title.y;
+        if(title.y === 'auto') title.y = 1;
         if(title.yanchor === 'auto') {
-            if(title.y < 0.5) {
-                title.yanchor = 'bottom';
-            } else {
-                title.yanchor = 'top';
-            }
+            title.yanchor = title.y < 0.5 ? 'bottom' : 'top';
         }
     }
 }
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -1981,15 +1981,15 @@ plots.doAutoMargin = function(gd) {
 
     var gs = fullLayout._size;
     var margin = fullLayout.margin;
-    var reservedMargins = {'t': 0, 'b': 0, 'l': 0, 'r': 0};
+    var reservedMargins = {t: 0, b: 0, l: 0, r: 0};
     var oldMargins = Lib.extendFlat({}, gs);
 
-    // We're assuming that each entry only pushes the margin out on one side
     var margins = gd._fullLayout._reservedMargin;
     for(var key in margins) {
-        var side = Object.keys(margins[key])[0];
-        var val = Object.values(margins[key])[0];
-        reservedMargins[side] += val;
+        for(var side in margins[key]) {
+            var val = margins[key][side];
+            reservedMargins[side] = Math.max(reservedMargins[side], val);
+        }
     }
     // adjust margins for outside components
     // fullLayout.margin is the requested margin,
@@ -2026,16 +2026,16 @@ plots.doAutoMargin = function(gd) {
             var pl = pushleft.size;
             var fb = pushbottom.val;
             var pb = pushbottom.size;
-            var nonReservedWidth = width - reservedMargins.r - reservedMargins.l;
-            var nonReservedHeight = height - reservedMargins.t - reservedMargins.b;
+            var availableWidth = width - reservedMargins.r - reservedMargins.l;
+            var availableHeight = height - reservedMargins.t - reservedMargins.b;
 
             for(var k2 in pushMargin) {
                 if(isNumeric(pl) && pushMargin[k2].r) {
                     var fr = pushMargin[k2].r.val;
                     var pr = pushMargin[k2].r.size;
                     if(fr > fl) {
-                        var newL = (pl * fr + (pr - nonReservedWidth) * fl) / (fr - fl);
-                        var newR = (pr * (1 - fl) + (pl - nonReservedWidth) * (1 - fr)) / (fr - fl);
+                        var newL = (pl * fr + (pr - availableWidth) * fl) / (fr - fl);
+                        var newR = (pr * (1 - fl) + (pl - availableWidth) * (1 - fr)) / (fr - fl);
                         if(newL + newR > ml + mr) {
                             ml = newL;
                             mr = newR;
@@ -2047,8 +2047,8 @@ plots.doAutoMargin = function(gd) {
                     var ft = pushMargin[k2].t.val;
                     var pt = pushMargin[k2].t.size;
                     if(ft > fb) {
-                        var newB = (pb * ft + (pt - nonReservedHeight) * fb) / (ft - fb);
-                        var newT = (pt * (1 - fb) + (pb - nonReservedHeight) * (1 - ft)) / (ft - fb);
+                        var newB = (pb * ft + (pt - availableHeight) * fb) / (ft - fb);
+                        var newT = (pt * (1 - fb) + (pb - availableHeight) * (1 - ft)) / (ft - fb);
                         if(newB + newT > mb + mt) {
                             mb = newB;
                             mt = newT;

Original file line number	Diff line number	Diff line change
`@@ -406,7 +406,7 @@ exports.drawMainTitle = function(gd) {`
`406`	`406`	`var dy = getMainTitleDy(fullLayout);`
`407`	`407`	`var y = getMainTitleY(fullLayout, dy);`
`408`	`408`
`409`		`- if(title.automargin) {`
	`409`	`+ if(title.text && title.automargin) {`
`410`	`410`	`applyTitleAutoMargin(gd, y);`
`411`	`411`	`}`
`412`	`412`
`@@ -442,23 +442,15 @@ function isOutsideContainer(gd, title, position, y) {`
`442`	`442`	`// 'auto' is not supported for either title.y or title.yanchor when automargin=true`
`443`	`443`	`function setDflts(title) {`
`444`	`444`	`if(title.automargin && title.yref === 'paper') {`
`445`		`- title.y = title.y === 0 ? title.y : 1;`
	`445`	`+ title.y = title.y === 0 ? 0 : 1;`
`446`	`446`	`if(title.yanchor === 'auto') {`
`447`		`- if(title.y === 0) {`
`448`		`- title.yanchor = 'top';`
`449`		`- } else {`
`450`		`- title.yanchor = 'bottom';`
`451`		`- }`
	`447`	`+ title.yanchor = title.y === 0 ? 'top' : 'bottom';`
`452`	`448`	`}`
`453`	`449`	`}`
`454`	`450`	`if(title.automargin && title.yref === 'container') {`
`455`		`- title.y = title.y === 'auto' ? 1 : title.y;`
	`451`	`+ if(title.y === 'auto') title.y = 1;`
`456`	`452`	`if(title.yanchor === 'auto') {`
`457`		`- if(title.y < 0.5) {`
`458`		`- title.yanchor = 'bottom';`
`459`		`- } else {`
`460`		`- title.yanchor = 'top';`
`461`		`- }`
	`453`	`+ title.yanchor = title.y < 0.5 ? 'bottom' : 'top';`
`462`	`454`	`}`
`463`	`455`	`}`
`464`	`456`	`}`