From 9f7a8e70534962e4bea13559c03766a5430eb582 Mon Sep 17 00:00:00 2001
From: Antoine Roy-Gobeil <antoine@plot.ly>
Date: Wed, 10 Oct 2018 11:41:46 -0400
Subject: [PATCH] improve autosize in edge case
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- provide fixed non-zero width/height when autosize:true and container's size is zero
- when setting container's widht/height, ignore autosize:true if width/height is explicitly specified by the user.
- 🔒 down in test that explicit width/height win over autosize:true
---
 src/plot_api/plot_api.js          |  7 +++---
 src/plot_api/subroutines.js       |  4 ++--
 test/jasmine/tests/config_test.js | 38 +++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js
index 609cf6cb3b2..a6875712fd5 100644
--- a/src/plot_api/plot_api.js
+++ b/src/plot_api/plot_api.js
@@ -488,6 +488,10 @@ function setPlotContext(gd, config) {
     if(context.setBackground === 'transparent' || typeof context.setBackground !== 'function') {
         context.setBackground = setBackground;
     }
+
+    // Check if gd has a specified widht/height to begin with
+    context._hasZeroHeight = context._hasZeroHeight || gd.clientHeight === 0;
+    context._hasZeroWidth = context._hasZeroWidth || gd.clientWidth === 0;
 }
 
 function plotPolar(gd, data, layout) {
@@ -3248,9 +3252,6 @@ function makePlotFramework(gd) {
     var gd3 = d3.select(gd);
     var fullLayout = gd._fullLayout;
 
-    // Check if gd has a specified height
-    fullLayout._hasZeroHeight = gd.clientHeight === 0;
-
     // Plot container
     fullLayout._container = gd3.selectAll('.plot-container').data([0]);
     fullLayout._container.enter().insert('div', ':first-child')
diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js
index 364762ba229..89521a227b1 100644
--- a/src/plot_api/subroutines.js
+++ b/src/plot_api/subroutines.js
@@ -54,8 +54,8 @@ function lsInner(gd) {
     var i, subplot, plotinfo, xa, ya;
 
     fullLayout._paperdiv.style({
-        width: (fullLayout.autosize) ? '100%' : fullLayout.width + 'px',
-        height: (fullLayout.autosize && !fullLayout._hasZeroHeight) ? '100%' : fullLayout.height + 'px'
+        width: (fullLayout.autosize && !gd._context._hasZeroWidth && !gd.layout.width) ? '100%' : fullLayout.width + 'px',
+        height: (fullLayout.autosize && !gd._context._hasZeroHeight && !gd.layout.height) ? '100%' : fullLayout.height + 'px'
     })
     .selectAll('.main-svg')
     .call(Drawing.setSize, fullLayout.width, fullLayout.height);
diff --git a/test/jasmine/tests/config_test.js b/test/jasmine/tests/config_test.js
index 44bc64bb07a..a7dc8a9dcab 100644
--- a/test/jasmine/tests/config_test.js
+++ b/test/jasmine/tests/config_test.js
@@ -174,6 +174,44 @@ describe('config argument', function() {
             .then(done);
         });
 
+        it('should provide a fixed non-zero width/height when autosize: true and container\' size is zero', function(done) {
+            gd.style.display = 'inline-block';
+            Plotly.plot(gd, data, {autosize: true})
+            .then(function() {
+                checkLayoutSize(700, 450);
+                expect(gd.clientWidth).toBe(700);
+                expect(gd.clientHeight).toBe(450);
+            })
+            .then(function() {
+                return Plotly.newPlot(gd, data, {autosize: true});
+            })
+            // It is important to test newPlot to make sure an initially zero size container
+            // is still considered to have zero size after a plot is drawn into it.
+            .then(function() {
+                checkLayoutSize(700, 450);
+                expect(gd.clientWidth).toBe(700);
+                expect(gd.clientHeight).toBe(450);
+            })
+            .catch(failTest)
+            .then(done);
+        });
+
+        // The following test is to guarantee we're not breaking the existing behaviour which may not be ideal.
+        // In a future version, one may prefer autosize:true winning over an explicit width/height when embedded in a webpage.
+        it('should use the explicitly provided width/height even if autosize:true', function(done) {
+            gd.style.width = '1000px';
+            gd.style.height = '500px';
+            Plotly.plot(gd, data, {autosize: true, width: 1200, height: 700})
+            .then(function() {
+                expect(gd.clientWidth).toBe(1000);
+                expect(gd.clientHeight).toBe(500);
+                // The plot should overflow the container!
+                checkLayoutSize(1200, 700);
+            })
+            .catch(failTest)
+            .then(done);
+        });
+
         it('should respect attribute autosizable: false', function(done) {
             var autosize = false;
             var config = {