Skip to content

Refactor plot autosize #635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 27, 2016
Merged

Refactor plot autosize #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
63ddd94
Revert "Revert "Remove internal option layout.autosize='initial' (Fix…
etpinard Jun 13, 2016
512679e
autosize: Fix Plots.resize backwards-compatibility
n-riesco Jun 12, 2016
b335b05
Merge branch 'master' into autosize-refactor
etpinard Oct 19, 2016
77971f4
make sure relayout(gd, 'width', null) resets width to default
etpinard Oct 19, 2016
b6c5610
plots: make autosize dflt *true* on layouts with no set width / height
etpinard Oct 26, 2016
4c14251
Merge branch 'master' into autosize-refactor
etpinard Oct 26, 2016
5fb7cd1
cache initial autosize vals -> use them on relayout
etpinard Oct 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ lib.getPlotDiv = function(el) {

lib.isPlotDiv = function(el) {
var el3 = d3.select(el);
return el3.size() && el3.classed('js-plotly-plot');
return el3.node() instanceof HTMLElement &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my own understanding: when would this not be an HTMLElement?

Copy link
Contributor Author

@etpinard etpinard Jun 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

el3.size() &&
el3.classed('js-plotly-plot');
};

lib.removeElement = function(el) {
Expand Down
125 changes: 22 additions & 103 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,6 @@ function plotPolar(gd, data, layout) {
if(layout) gd.layout = layout;
Polar.manager.fillLayout(gd);

if(gd._fullLayout.autosize === 'initial' && gd._context.autosizable) {
plotAutoSize(gd, {});
gd._fullLayout.autosize = layout.autosize = true;
}
// resize canvas
paperDiv.style({
width: gd._fullLayout.width + 'px',
Expand Down Expand Up @@ -1778,8 +1774,6 @@ function _relayout(gd, aobj) {
var redoit = {},
undoit = {};

var hw = ['height', 'width'];

// for attrs that interact (like scales & autoscales), save the
// old vals before making the change
// val=undefined will not set a value, just record what the value was.
Expand Down Expand Up @@ -1827,13 +1821,14 @@ function _relayout(gd, aobj) {
// op and has no flag.
undoit[ai] = (pleaf === 'reverse') ? vi : p.get();

// check autosize or autorange vs size and range
if(hw.indexOf(ai) !== -1) {
doextra('autosize', false);
}
else if(ai === 'autosize') {
doextra(hw, undefined);
// Setting width or height to null must reset the graph's width / height
// back to its initial value as computed during the first pass in Plots.plotAutoSize.
//
// To do so, we must manually set them back here using the _initialAutoSize cache.
if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
gd._fullLayout[ai] = gd._initialAutoSize[ai];
}
// check autorange vs range
else if(pleafPlus.match(/^[xyz]axis[0-9]*\.range(\[[0|1]\])?$/)) {
doextra(ptrunk + '.autorange', false);
}
Expand Down Expand Up @@ -2017,10 +2012,21 @@ function _relayout(gd, aobj) {
}
}

// calculate autosizing - if size hasn't changed,
// will remove h&w so we don't need to redraw
if(aobj.autosize) aobj = plotAutoSize(gd, aobj);
if(aobj.height || aobj.width || aobj.autosize) flags.docalc = true;
var oldWidth = gd._fullLayout.width,
oldHeight = gd._fullLayout.height;

// coerce the updated layout
Plots.supplyDefaults(gd);

// calculate autosizing
if(gd.layout.autosize) Plots.plotAutoSize(gd, gd.layout, gd._fullLayout);

// avoid unnecessary redraws
var hasSizechanged = aobj.height || aobj.width ||
(gd._fullLayout.width !== oldWidth) ||
(gd._fullLayout.height !== oldHeight);

if(hasSizechanged) flags.docalc = true;

if(flags.doplot || flags.docalc) {
flags.layoutReplot = true;
Expand Down Expand Up @@ -2616,86 +2622,6 @@ Plotly.purge = function purge(gd) {
return gd;
};

/**
* 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;
}

function plotAutoSize(gd, aobj) {
var fullLayout = gd._fullLayout,
context = gd._context,
computedStyle;

var newHeight, newWidth;

gd.emit('plotly_autosize');

// embedded in an iframe - just take the full iframe size
// if we get to this point, with no aspect ratio restrictions
if(gd._context.fillFrame) {
newWidth = window.innerWidth;
newHeight = window.innerHeight;

// somehow we get a few extra px height sometimes...
// just hide it
document.body.style.overflow = 'hidden';
}
else if(isNumeric(context.frameMargins) && context.frameMargins > 0) {
var reservedMargins = calculateReservedMargins(gd._boundingBoxMargins),
reservedWidth = reservedMargins.left + reservedMargins.right,
reservedHeight = reservedMargins.bottom + reservedMargins.top,
gdBB = fullLayout._container.node().getBoundingClientRect(),
factor = 1 - 2 * context.frameMargins;

newWidth = Math.round(factor * (gdBB.width - reservedWidth));
newHeight = Math.round(factor * (gdBB.height - reservedHeight));
}
else {
// plotly.js - let the developers do what they want, either
// provide height and width for the container div,
// specify size in layout, or take the defaults,
// but don't enforce any ratio restrictions
computedStyle = window.getComputedStyle(gd);
newHeight = parseFloat(computedStyle.height) || fullLayout.height;
newWidth = parseFloat(computedStyle.width) || fullLayout.width;
}

if(Math.abs(fullLayout.width - newWidth) > 1 ||
Math.abs(fullLayout.height - newHeight) > 1) {
fullLayout.height = gd.layout.height = newHeight;
fullLayout.width = gd.layout.width = newWidth;
}
// if there's no size change, update layout but
// delete the autosize attr so we don't redraw
// but can't call layoutStyles for initial autosize
else if(fullLayout.autosize !== 'initial') {
delete(aobj.autosize);
fullLayout.autosize = gd.layout.autosize = true;
}

Plots.sanitizeMargins(fullLayout);

return aobj;
}

// -------------------------------------------------------
// makePlotFramework: Create the plot container and axes
// -------------------------------------------------------
Expand All @@ -2715,13 +2641,6 @@ function makePlotFramework(gd) {
.classed('svg-container', true)
.style('position', 'relative');

// Initial autosize
if(fullLayout.autosize === 'initial') {
plotAutoSize(gd, {});
fullLayout.autosize = true;
gd.layout.autosize = true;
}

// Make the graph containers
// start fresh each time we get here, so we know the order comes out
// right, rather than enter/exit which can muck up the order
Expand Down
7 changes: 4 additions & 3 deletions src/plot_api/plot_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ module.exports = {
// we can edit titles, move annotations, etc
editable: false,

// DO autosize once regardless of layout.autosize
// (use default width or height values otherwise)
autosizable: false,

// set the length of the undo/redo queue
queueLength: 0,

// plot will respect layout.autosize=true and infer its container size
autosizable: false,

// if we DO autosize, do we fill the container or the screen?
fillFrame: false,

Expand Down
14 changes: 9 additions & 5 deletions src/plots/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@ module.exports = {
description: 'Sets the title font.'
}),
autosize: {
valType: 'enumerated',
valType: 'boolean',
role: 'info',
// TODO: better handling of 'initial'
values: [true, false, 'initial'],
dflt: false,
description: [
'Determines whether or not the dimensions of the figure are',
'computed as a function of the display size.'
'Determines whether or not a layout width or height',
'that has been left undefined by the user',
'is initialized on each relayout.',

Copy link
Contributor

@mdtusz mdtusz Jun 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐄 ?

Not sure if intentional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this wording could be improved. The most important points I wanted to make in the description were that:

  • if the user supplies values of layout.width and/or layout.height, plotAutoSize doesn't change them.
  • if layout.autosize is false, plotAutosize is called once.
  • if layout.autosize is true, plotAutosize is called on each relayout.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was meaning just the blank line. I think the description itself is clear enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T'was intentional 😄 but if it looks like a 🐮, it must be one.

'Note that, regardless of this attribute,',
'an undefined layout width or height',
'is always initialized on the first call to plot.'
].join(' ')
},
width: {
Expand Down
Loading