Skip to content

newPlot doesn't respect layout.height unless layout.autosize is explicitly true #537

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.

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

Closed
cpsievert opened this issue May 15, 2016 · 5 comments · Fixed by #635
Closed

newPlot doesn't respect layout.height unless layout.autosize is explicitly true #537

cpsievert opened this issue May 15, 2016 · 5 comments · Fixed by #635
Assignees
Labels
bug something broken
Milestone

Comments

@cpsievert
Copy link

I know this should really be done with relayout, but nevertheless, I found the behavior surprising.

http://codepen.io/cpsievert/pen/yOrQya

@etpinard
Copy link
Contributor

I would definitively call this a bug. Thanks for bringing this up.

The root of the problem is in this block where layouts with incomplete size specifications (denoted by autosize: 'initial' internally) go through the plotAutoSize routine which overrides the user layout spec by the computed size of the graph div.

I believe the aforementioned block should instead be:

computedStyle = window.getComputedStyle(gd);
newHeight = fullLayout.height || parseFloat(computedStyle.height);
newWidth = fullLayout.width || parseFloat(computedStyle.width);

@n-riesco
Copy link
Contributor

@etpinard While looking into #106 and #272 I came across this TODO and I'm looking into removing 'initial' from autosize. Since it touches the same code, I could address this issue in the same PR.

@etpinard etpinard assigned etpinard and n-riesco and unassigned etpinard May 25, 2016
@n-riesco
Copy link
Contributor

I've also found that autosize: true isn't honoured in the following cases:

  • when Plotly.plot(Tabs.fresh(), [{ x:[1,2,3], y:[3,2,1]}], {autosize: true}) is run, the graph is given the default dimensions 700x450 regardless of the dimensions of the container. Only after Plotly.relayout(gd, {autosize: true}) is run, the graph takes the dimensions of the container;
  • Plotly.relayout(gd, {}) has no effect.

In the PR I'm preparing, I'm planning to ensure that:

  • Plotly.plot(Tabs.fresh(), [{ x:[1,2,3], y:[3,2,1]}], {autosize: true}) creates a graph with the dimensions of the container;
  • and that Plotly.relayout(gd, {}) triggers plotAutoSize if autosize is set.

n-riesco added a commit to n-riesco/plotly.js that referenced this issue May 27, 2016
* Moved initial call to `plotAutoSize` into `Plots.supplyDefaults(gd)`.

* Replaced `{ autosize: 'initial' }` with the flag
  `gd._fullLayout._initialAutoSizeIsDone`.

* `{ autosize: false }` the values of width and height undefined in
  `gd.layout` will be autosized only once.

* `{ autosize: true }` only autosizes the values of width and height
  undefined in `gd.layout`.

Fixes plotly#537
n-riesco added a commit to n-riesco/plotly.js that referenced this issue May 27, 2016
* Added test to check `Plotly.newPlot` respects `layout.width` and
  `layout.height`.
n-riesco added a commit to n-riesco/plotly.js that referenced this issue May 27, 2016
* Moved initial call to `plotAutoSize` into `Plots.supplyDefaults(gd)`.

* Replaced `{ autosize: 'initial' }` with the flag
  `gd._fullLayout._initialAutoSizeIsDone`.

* `{ autosize: false }` the values of width and height undefined in
  `gd.layout` will be autosized only once.

* `{ autosize: true }` only autosizes the values of width and height
  undefined in `gd.layout`.

Fixes plotly#537
n-riesco added a commit to n-riesco/plotly.js that referenced this issue May 27, 2016
* Added test to check `Plotly.newPlot` respects `layout.width` and
  `layout.height`.
etpinard added a commit that referenced this issue Jun 9, 2016
Remove internal option layout.autosize='initial' (Fixes #537)
etpinard added a commit that referenced this issue Jun 13, 2016
Revert "Remove internal option layout.autosize='initial' (Fixes #537)"
@cpsievert
Copy link
Author

cpsievert commented Sep 1, 2016

It looks like the issue I originally reported still persists?

@cpsievert cpsievert reopened this Sep 1, 2016
@etpinard
Copy link
Contributor

etpinard commented Sep 1, 2016

It looks like the issue I originally reported still persists?

Correct. Will be done in #635

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants