Merge pull request #3090 from plotly/fix-autosize

improve autosize in edge cases
plotly · Oct 10, 2018 · cc21e5c · cc21e5c
2 parents 3f7a2d3 + af4a2ce
commit cc21e5c
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 6 deletions.
diff --git 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/plot_config.js b/src/plot_api/plot_config.js
@@ -57,7 +57,10 @@ module.exports = {
      */
     autosizable: false,
 
-    // responsive: determines whether to change the layout size when window is resized
+    /*
+     * responsive: determines whether to change the layout size when window is resized.
+     * In v2, this option will be removed and will always be true.
+     */
     responsive: false,
 
     // set the length of the undo/redo queue

diff --git 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: (gd._context.responsive && fullLayout.autosize && !gd._context._hasZeroWidth && !gd.layout.width) ? '100%' : fullLayout.width + 'px',
+        height: (gd._context.responsive && 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
@@ -705,5 +705,50 @@ describe('config argument', function() {
             .then(testResponsive)
             .then(done);
         });
+
+        it('should provide a fixed non-zero width/height when autosize/responsive: true and container\' size is zero', function(done) {
+            fillParent(1, 1, function() {
+                this.style.display = 'inline-block';
+                this.style.width = null;
+                this.style.height = null;
+            });
+            Plotly.plot(gd, data, {autosize: true}, {responsive: 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}, {responsive: 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 (although maybe not ideal) behaviour.
+        // In a future version, one may prefer responsive/autosize:true winning over an explicit width/height when embedded in a webpage.
+        it('should use the explicitly provided width/height even if autosize/responsive:true', function(done) {
+            fillParent(1, 1, function() {
+                this.style.width = '1000px';
+                this.style.height = '500px';
+            });
+
+            Plotly.plot(gd, data, {autosize: true, width: 1200, height: 700}, {responsive: true})
+            .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);
+        });
     });
 });