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

Fix issues rendering shiny runtime Rmds from rmdshot() #53

Merged
merged 4 commits into from
Jul 17, 2023
Merged

Conversation

gadenbuie
Copy link
Member

Fixes #52:

  • Pass rmd_args to rmarkdown::run(render_args =)
  • Pass p to wait_until_server_exists(), without which an error is thrown.

@gadenbuie gadenbuie requested review from wch and schloerke July 17, 2023 17:54
@wch wch merged commit 36debdc into main Jul 17, 2023
@wch wch deleted the fix/rmdshot_shiny branch July 17, 2023 22:32
@wch
Copy link
Collaborator

wch commented Jul 17, 2023

Thanks for the fix!

@@ -1,3 +1,7 @@
# webshot2 (development version)

* Fixed `rmdshot()` when used to screenshot an R Markdown document with `runtime: shiny` or `runtime: shinyrmd` (#52).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just merged but then realized that this doesn't quite have the same format as other NEWS items. It should be

* Fixed #52: `rmdshot()` did not work when used to screenshot an R Markdown document with `runtime: shiny` or `runtime: shinyrmd`. (#53)

I'll fix on main.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 but there aren't any other NEWS items in this repo.

Is this style guidance written down somewhere? It would be helpful, both internally and externally if it were. And along those lines, while I can live with us deviating from the tidyverse style, it'd be easier for external collaborators and new contributors if we adopted the tidyverse style guidelines.

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.

rmdshot doesn't work when i add a params argument
2 participants