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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 4 commits into from
Jul 17, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
595197d
fix(rmdshot_shiny): Correctly pass `rmd_args` to `render_args`
gadenbuie Jul 17, 2023
d9ad004
fix(rmdshot_shiny): Pass process object `p`
gadenbuie Jul 17, 2023
674c7c2
Increment version number to 0.1.0.9000
gadenbuie Jul 17, 2023
b360fc4
docs: Add NEWS item
gadenbuie Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: webshot2
Title: Take Screenshots of Web Pages
Version: 0.1.0
Version: 0.1.0.9000
Authors@R: c(
person("Winston", "Chang", role = c("aut", "cre"), email = "winston@rstudio.com"),
person("Barret", "Schloerke", role = c("ctb"), email = "barret@rstudio.com", comment = c(ORCID = "0000-0001-9986-114X")),
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.


# webshot2 0.1.0

* Added a `NEWS.md` file to track changes to the package.
9 changes: 5 additions & 4 deletions R/rmdshot.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ rmdshot_shiny <- function(doc, file, ..., rmd_args, port, envvars) {
function(...) {
rmarkdown::run(...)
},
args = append(
list(file = doc, shiny_args = list(port = port)),
rmd_args
args = list(
file = doc,
shiny_args = list(port = port),
render_args = rmd_args
),
envvars = envvars
)
Expand All @@ -75,7 +76,7 @@ rmdshot_shiny <- function(doc, file, ..., rmd_args, port, envvars) {
})

# Wait for app to start
wait_until_server_exists(url)
wait_until_server_exists(url, p)

fileout <- webshot(url, file = file, ...)

Expand Down