Skip to content

Commit

Permalink
only set container's size to 100% when responsive:true
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
antoinerg committed Oct 10, 2018
1 parent 9f7a8e7 commit af4a2ce
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 41 deletions.
5 changes: 4 additions & 1 deletion src/plot_api/plot_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 && !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);
Expand Down
83 changes: 45 additions & 38 deletions test/jasmine/tests/config_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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);
});
});
});

0 comments on commit af4a2ce

Please sign in to comment.