Skip to content

RFC: virtual width/height for ggplotly() #623

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
wants to merge 1 commit into from

Conversation

alyst
Copy link
Contributor

@alyst alyst commented Jun 6, 2016

This is the proof-of-concept workaround for the layout problems of ggplotly() (see e.g. #622).
The problem is that when converting from absolute sizes of e.g. labels ("inches", "mm" etc) to relative sizes ("npc") ggplotly uses grid::convertXXX() functions that use the dimensions of the active device, which may not match the size of the plotly plot on the client.
The PR provides at least the way to control the size of the active device on the server side. It introduces virt.width/virt.height arguments to ggplotly() that serve as the expected dimensions of the client plot. If set to numeric values, ggplotly() creates the viewport of the specified dimensions (in "points"), so that all conversions to/from "npc" are done relative to this viewport. virt.width/height do not affect the size of the resulting plot, neither they interfere with the width/height (which might be set to something not numerical as e.g. "100%").

specify the "virtual" dimensions of the plot to control
the "pixels" <-> "npc" conversion
@talgalili
Copy link
Contributor

Hi @alyst
Does this help solve #600 ?

Also, would this make the margin in the following work (without the need to use layout):

library(heatmaply)
heatmaply(mtcars)

?

Thanks,
Tal

@alyst
Copy link
Contributor Author

alyst commented Jun 6, 2016

@talgalili It might help, although (running plotly with the other layout patches I've recently submitted) I cannot reproduce ticks labels clipping.

I don't know if the PR would help heatmaply. Currently virtual viewport workaround is implemented for ggplotly() variant that accepts ggplot objects. It's very easy to extend the PR to the other plot types, but if heatmaply is not inherited from ggplot the current version would do nothing.

@talgalili
Copy link
Contributor

@alyst heatmaply works using ggplotly (using "geom_tile"), so I think it might help. If you'll get to check it (and it helps) - please let me know.

Best,
Tal

@cpsievert
Copy link
Collaborator

This is a nice idea, thank @alyst, sorry for the delay.

Why do u set defaults of 768 and 1024 for height/width?

@cpsievert
Copy link
Collaborator

I don't think we need to add additional arguments... see 9afbb5d

Thanks for the idea @alyst!

@cpsievert cpsievert closed this Sep 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants