From af4a2cefe10de4eb855f31c9503cb87e68458a90 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Wed, 10 Oct 2018 13:47:36 -0400 Subject: [PATCH] only set container's size to 100% when responsive:true This change will prevent any existing applications from breaking. Anyway setting the container's size to 100% was only useful for responsiveness in modern CSS layouts. --- src/plot_api/plot_config.js | 5 +- src/plot_api/subroutines.js | 4 +- test/jasmine/tests/config_test.js | 83 +++++++++++++++++-------------- 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/src/plot_api/plot_config.js b/src/plot_api/plot_config.js index 574b618e0a7..a3751322ec5 100644 --- 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 index 89521a227b1..07445cd3816 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 && !gd._context._hasZeroWidth && !gd.layout.width) ? '100%' : fullLayout.width + 'px', - height: (fullLayout.autosize && !gd._context._hasZeroHeight && !gd.layout.height) ? '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 index a7dc8a9dcab..e85451e6a5d 100644 --- a/test/jasmine/tests/config_test.js +++ b/test/jasmine/tests/config_test.js @@ -174,44 +174,6 @@ 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 = { @@ -743,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); + }); }); });