Skip to content
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

Suppress immediate condformat2grob output #30

Closed
interzoneboy opened this issue Nov 24, 2022 · 3 comments
Closed

Suppress immediate condformat2grob output #30

interzoneboy opened this issue Nov 24, 2022 · 3 comments

Comments

@interzoneboy
Copy link

I note that when I call condformat2grob the plot is immediately generated and displayed, even if what I'm trying to do is to store the grob in a variable for layout with ggarrange or grid.arrange. In my case, where I have two conditionally formatted tables and one ggplot layed out with ggarrange, this causes knitr to output first the one table, then the second one, and then the composite figure (which itself is just fine).

I seem to have fixed this myself by altering the source code -- I commented out two function calls right at the end of the condformat2grob function: grid::grid.newpage() and grid::grid.draw(gridobj). The subsequent call (invisible(gridobj)) seems to return the grob properly, and my ggarrange layout works as it should.

Knocking out these two lines probably messed up something elsewhere (so not going to make a PR out of this), but for my purposes it works perfectly. Thanks for your work on this -- it's fantastic!

@zeehio
Copy link
Owner

zeehio commented Nov 24, 2022

Hi! Thanks for your feedback. It's nice to know the package has users!

I believe your solution is actually a very good approach.

If you want to create a pull request, I would review and merge one that:

  • Adds a draw=TRUE argument to condformat2grob().
  • puts the grid.newpage() and grid.draw() calls within an if (draw) {...}
  • Adds the corresponding documentation
  • Adds a NEWS entry

Would you be up to do that?

@interzoneboy
Copy link
Author

Yes indeed. Pull request submitted #31

@zeehio zeehio closed this as completed in 0d51658 Nov 26, 2022
@zeehio
Copy link
Owner

zeehio commented Nov 26, 2022

Merged and on its way to CRAN 👍

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

No branches or pull requests

2 participants