From 87d47f35c508b02a836541d2a44a6ce99316c37d Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 27 Sep 2018 16:33:41 -0400 Subject: [PATCH 1/4] fix css style of container to properly resize in flexbox and grid --- src/plot_api/plot_api.js | 3 + src/plot_api/subroutines.js | 4 +- src/plots/plots.js | 9 ++- test/jasmine/tests/config_test.js | 95 +++++++++++++++++++++++++------ 4 files changed, 87 insertions(+), 24 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 4fe0a28da59..d0bd011f9b3 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -3195,6 +3195,9 @@ 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 aa185cbf328..b0c0dbdef21 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -105,8 +105,8 @@ function lsInner(gd) { fullLayout._paperdiv .style({ - width: fullLayout.width + 'px', - height: fullLayout.height + 'px' + width: (fullLayout.autosize) ? '100%' : fullLayout.width + 'px', + height: (fullLayout.autosize && !fullLayout._hasZeroHeight) ? '100%' : fullLayout.height + 'px' }) .selectAll('.main-svg') .call(Drawing.setSize, fullLayout.width, fullLayout.height); diff --git a/src/plots/plots.js b/src/plots/plots.js index 8f172d14ba0..3025a4f6982 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1371,11 +1371,10 @@ plots.plotAutoSize = function plotAutoSize(gd, layout, fullLayout) { reservedHeight = reservedMargins.bottom + reservedMargins.top, factor = 1 - 2 * frameMargins; - var gdBB = fullLayout._container && fullLayout._container.node ? - fullLayout._container.node().getBoundingClientRect() : { - width: fullLayout.width, - height: fullLayout.height - }; + var gdBB = { + width: fullLayout.width, + height: fullLayout.height + }; newWidth = Math.round(factor * (gdBB.width - reservedWidth)); newHeight = Math.round(factor * (gdBB.height - reservedHeight)); diff --git a/test/jasmine/tests/config_test.js b/test/jasmine/tests/config_test.js index b20814ceaa8..c8497b36e2b 100644 --- a/test/jasmine/tests/config_test.js +++ b/test/jasmine/tests/config_test.js @@ -543,23 +543,26 @@ describe('config argument', function() { }); describe('responsive figure', function() { - var gd; - var startWidth = 960, startHeight = 400; - var newWidth = 400, newHeight = 700; - var data = [{x: [1, 2, 3, 4], y: [5, 10, 2, 8]}]; + var gd, data = [{x: [1, 2, 3, 4], y: [5, 10, 2, 8]}]; + var width = 960, height = 800; + + var parent, elWidth, elHeight; beforeEach(function() { - viewport.set(startWidth, startHeight); - gd = createGraphDiv(); + viewport.set(width, height); - // Make the graph fill the parent - gd.style.width = '100%'; - gd.style.height = '100%'; + // Prepare a parent container that fills the viewport + parent = document.createElement('div'); + parent.style.width = '100vw'; + parent.style.height = '100vh'; + parent.style.position = 'fixed'; + parent.style.top = '0'; + parent.style.left = '0'; }); afterEach(function() { Plotly.purge(gd); // Needed to remove all event listeners - destroyGraphDiv(); + document.body.removeChild(parent); viewport.reset(); }); @@ -573,24 +576,41 @@ describe('config argument', function() { } function testResponsive() { - checkLayoutSize(startWidth, startHeight); - viewport.set(newWidth, newHeight); + checkLayoutSize(elWidth, elHeight); + viewport.set(width / 2, height / 2); return Promise.resolve() .then(delay(200)) .then(function() { - checkLayoutSize(newWidth, newHeight); + checkLayoutSize(elWidth / 2, elHeight / 2); }) .catch(failTest); } + function fillParent(numRows, numCols, cb) { + elWidth = width / numCols, elHeight = height / numRows; + + // Fill parent + for(var i = 0; i < (numCols * numRows); i++) { + var col = document.createElement('div'); + col.style.height = '100%'; + col.style.width = '100%'; + if(typeof(cb) === typeof(Function)) cb.call(col, i); + parent.appendChild(col); + } + document.body.appendChild(parent); + gd = parent.childNodes[0]; + } + it('should resize when the viewport width/height changes', function(done) { + fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(testResponsive) .then(done); }); it('should still be responsive if the plot is edited', function(done) { + fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.restyle(gd, 'y[0]', data[0].y[0] + 2);}) .then(testResponsive) @@ -598,6 +618,7 @@ describe('config argument', function() { }); it('should still be responsive if the plot is purged and replotted', function(done) { + fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.newPlot(gd, data, {}, {responsive: true});}) .then(testResponsive) @@ -605,13 +626,14 @@ describe('config argument', function() { }); it('should only have one resize handler when plotted more than once', function(done) { + fillParent(1, 1); var cntWindowResize = 0; window.addEventListener('resize', function() {cntWindowResize++;}); spyOn(Plotly.Plots, 'resize').and.callThrough(); Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.restyle(gd, 'y[0]', data[0].y[0] + 2);}) - .then(function() {viewport.set(newWidth, newHeight);}) + .then(function() {viewport.set(width / 2, width / 2);}) .then(delay(200)) // .then(function() {viewport.set(newWidth, 2 * newHeight);}).then(delay(200)) .then(function() { @@ -623,6 +645,7 @@ describe('config argument', function() { }); it('should become responsive if configured as such via Plotly.react', function(done) { + fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: false}) .then(function() {return Plotly.react(gd, data, {}, {responsive: true});}) .then(testResponsive) @@ -630,19 +653,57 @@ describe('config argument', function() { }); it('should stop being responsive if configured as such via Plotly.react', function(done) { + fillParent(1, 1); Plotly.plot(gd, data, {}, {responsive: true}) // Check initial size - .then(function() {checkLayoutSize(startWidth, startHeight);}) + .then(function() {checkLayoutSize(width, height);}) // Turn off responsiveness .then(function() {return Plotly.react(gd, data, {}, {responsive: false});}) // Resize viewport - .then(function() {viewport.set(newWidth, newHeight);}) + .then(function() {viewport.set(width / 2, height / 2);}) // Wait for resize to happen (Plotly.resize has an internal timeout) .then(delay(200)) // Check that final figure's size hasn't changed - .then(function() {checkLayoutSize(startWidth, startHeight);}) + .then(function() {checkLayoutSize(width, height);}) .catch(failTest) .then(done); }); + + // Testing fancier CSS layouts + it('should resize horizontally in a flexbox when responsive: true', function(done) { + parent.style.display = 'flex'; + parent.style.flexDirection = 'row'; + fillParent(1, 2, function() { + this.style.flexGrow = '1'; + }); + + Plotly.plot(gd, data, {}, { responsive: true }) + .then(testResponsive) + .then(done); + }); + + it('should resize vertically in a flexbox when responsive: true', function(done) { + parent.style.display = 'flex'; + parent.style.flexDirection = 'column'; + fillParent(2, 1, function() { + this.style.flexGrow = '1'; + }); + + Plotly.plot(gd, data, {}, { responsive: true }) + .then(testResponsive) + .then(done); + }); + + it('should resize in both direction in a grid when responsive: true', function(done) { + var numCols = 2, numRows = 2; + parent.style.display = 'grid'; + parent.style.gridTemplateColumns = 'repeat(' + numCols + ', 1fr)'; + parent.style.gridTemplateRows = 'repeat(' + numRows + ', 1fr)'; + fillParent(numRows, numCols); + + Plotly.plot(gd, data, {}, { responsive: true }) + .then(testResponsive) + .then(done); + }); }); }); From 95d67ea2cd1050ce4f131a58701c1caf884b2740 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Thu, 27 Sep 2018 18:10:00 -0400 Subject: [PATCH 2/4] ensure figure has fixed size for testing hover --- test/jasmine/tests/hover_label_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index 75e963c78d9..ed598ab3b99 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1967,7 +1967,7 @@ describe('hover updates', function() { size: 16, color: colors0.slice() } - }]) + }], { width: 700, height: 450 }) .then(function() { gd.on('plotly_hover', function(eventData) { From b3a4f23155a953d780422c9f5e964b4be93b23e3 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Fri, 28 Sep 2018 11:27:40 -0400 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=94=AA=20calculatedReserverdMargins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/plots/plots.js | 38 +++----------------------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 3025a4f6982..59a49ab7ba7 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1366,18 +1366,9 @@ plots.plotAutoSize = function plotAutoSize(gd, layout, fullLayout) { document.body.style.overflow = 'hidden'; } else if(isNumeric(frameMargins) && frameMargins > 0) { - var reservedMargins = calculateReservedMargins(gd._boundingBoxMargins), - reservedWidth = reservedMargins.left + reservedMargins.right, - reservedHeight = reservedMargins.bottom + reservedMargins.top, - factor = 1 - 2 * frameMargins; - - var gdBB = { - width: fullLayout.width, - height: fullLayout.height - }; - - newWidth = Math.round(factor * (gdBB.width - reservedWidth)); - newHeight = Math.round(factor * (gdBB.height - reservedHeight)); + var factor = 1 - 2 * frameMargins; + newWidth = Math.round(factor * fullLayout.width); + newHeight = Math.round(factor * fullLayout.height); } else { // plotly.js - let the developers do what they want, either @@ -1414,29 +1405,6 @@ plots.plotAutoSize = function plotAutoSize(gd, layout, fullLayout) { plots.sanitizeMargins(fullLayout); }; -/** - * Reduce all reserved margin objects to a single required margin reservation. - * - * @param {Object} margins - * @returns {{left: number, right: number, bottom: number, top: number}} - */ -function calculateReservedMargins(margins) { - var resultingMargin = {left: 0, right: 0, bottom: 0, top: 0}, - marginName; - - if(margins) { - for(marginName in margins) { - if(margins.hasOwnProperty(marginName)) { - resultingMargin.left += margins[marginName].left || 0; - resultingMargin.right += margins[marginName].right || 0; - resultingMargin.bottom += margins[marginName].bottom || 0; - resultingMargin.top += margins[marginName].top || 0; - } - } - } - return resultingMargin; -} - plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData, transitionData) { var componentsRegistry = Registry.componentsRegistry; var basePlotModules = layoutOut._basePlotModules; From 427ffe067c0a66c7ef432eef2defd66a41c10c3a Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Fri, 28 Sep 2018 12:29:09 -0400 Subject: [PATCH 4/4] frameMargins evaluated with respect to div's size --- src/plots/plots.js | 11 ++++++----- test/jasmine/tests/config_test.js | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 59a49ab7ba7..383a07b91c9 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1365,11 +1365,6 @@ plots.plotAutoSize = function plotAutoSize(gd, layout, fullLayout) { // just hide it document.body.style.overflow = 'hidden'; } - else if(isNumeric(frameMargins) && frameMargins > 0) { - var factor = 1 - 2 * frameMargins; - newWidth = Math.round(factor * fullLayout.width); - newHeight = Math.round(factor * fullLayout.height); - } else { // plotly.js - let the developers do what they want, either // provide height and width for the container div, @@ -1379,6 +1374,12 @@ plots.plotAutoSize = function plotAutoSize(gd, layout, fullLayout) { newWidth = parseFloat(computedStyle.width) || parseFloat(computedStyle.maxWidth) || fullLayout.width; newHeight = parseFloat(computedStyle.height) || parseFloat(computedStyle.maxHeight) || fullLayout.height; + + if(isNumeric(frameMargins) && frameMargins > 0) { + var factor = 1 - 2 * frameMargins; + newWidth = Math.round(factor * newWidth); + newHeight = Math.round(factor * newHeight); + } } var minWidth = plots.layoutAttributes.width.min, diff --git a/test/jasmine/tests/config_test.js b/test/jasmine/tests/config_test.js index c8497b36e2b..44bc64bb07a 100644 --- a/test/jasmine/tests/config_test.js +++ b/test/jasmine/tests/config_test.js @@ -145,7 +145,7 @@ describe('config argument', function() { fillFrame: false, frameMargins: 0.1 }; - var layoutHeight = 360; + var layoutHeight = Math.round(0.8 * containerHeightBeforePlot); var relayoutHeight = layoutHeight; testAutosize(autosize, config, layoutHeight, relayoutHeight, done); @@ -157,8 +157,8 @@ describe('config argument', function() { fillFrame: false, frameMargins: 0.1 }; - var layoutHeight = 360; - var relayoutHeight = 288; + var layoutHeight = Math.round(0.8 * containerHeightBeforePlot); + var relayoutHeight = Math.round(0.8 * containerHeightBeforeRelayout); testAutosize(autosize, config, layoutHeight, relayoutHeight, done); });