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

Add knit_print method for webshot #27

Merged
merged 4 commits into from
Aug 19, 2016
Merged

Add knit_print method for webshot #27

merged 4 commits into from
Aug 19, 2016

Conversation

jimhester
Copy link
Contributor

This lets you include webshot() calls directly in knitted documents and automatically show the output image.

The object must be returned visibly in order to show up when knitting, so I had to define a print method as well to preserve the previous behavior.

@wch
Copy link
Owner

wch commented Aug 19, 2016

I think you can avoid importing knitr. See https://github.com/ramnathv/htmlwidgets/blob/master/R/knitr-methods.R for an example.

@jimhester
Copy link
Contributor Author

I actually had seen that code when I was doing this originally, thought you might not want the added complexity.

8ae7e27 keeps knitr in Suggests and uses the referenced functions.

S3method(print,webshot)
export("%>%")
export(appshot)
export(install_phantomjs)
export(knit_print.webshot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this does not need to be exported.

@wch
Copy link
Owner

wch commented Aug 19, 2016

Could you also update the functions in image.R to check for the webshot class and also return a filename with that class? Otherwise, I think that doing something like webshot() %>% resize() in a knitr document won't result in the image being put inline.

@jimhester
Copy link
Contributor Author

@wch Ok both shrink and resize now also return webshot objects as well.

@wch wch merged commit eef27e7 into wch:master Aug 19, 2016
@wch
Copy link
Owner

wch commented Aug 19, 2016

Thanks!

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