Skip to content

Commit

Permalink
improve autosize in edge case
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
antoinerg committed Oct 10, 2018
1 parent 5cc37e3 commit 9f7a8e7
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 5 deletions.
7 changes: 4 additions & 3 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 38 additions & 0 deletions test/jasmine/tests/config_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down

0 comments on commit 9f7a8e7

Please sign in to comment.