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

Support directly including htmlDependency() objects in rmarkdown #376

Merged
merged 5 commits into from May 15, 2023

Conversation

gadenbuie
Copy link
Member

This PR fixes #375, making it possible to include an htmlDependency() simply by calling it in an R chunk where it's handled by knit_print().

Previously, users had to include the dependency in a tagList() or a tag to use the knit_print.shiny.tag method. Fortunately, that same method works for htmlDependency() objects too.

This small R Markdown document will now show the pencil icon, where previously it would print the font awesome dependency and no icon would be visible.

---
title: Issue 375
---

```{r}
library(htmltools)

fontawesome::fa_html_dependency()
tags$i(class = "fas fa-pencil")
```


```{r}
print(fontawesome::fa_html_dependency())
```

This is a slightly breaking change in the sense that users who were previously printing the html dependency will now need to use print() explicitly to show the object structure. In general, I think the new behavior is preferable.

since knitr is in Enhances
@gadenbuie
Copy link
Member Author

@schloerke I added skip_if_not_installed("knitr") to the test I added around this change, should we ensure knitr is installed in CI for r-cmd-check? It currently isn't due to knitr being an Enhances dependency

@schloerke
Copy link
Collaborator

Good catch!

@schloerke
Copy link
Collaborator

Yes, let's add it in Config/needs/check: knitr in the DESCRIPTION file. (Similar to Config/needs/website)

Copy link
Contributor

@cderv cderv left a comment

Choose a reason for hiding this comment

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

LGTM too. I think this will be easier to use now.

It seems like a justified breaking change to me.

@gadenbuie gadenbuie requested a review from cpsievert May 15, 2023 15:31
@cpsievert cpsievert merged commit 73fd307 into main May 15, 2023
15 checks passed
@cpsievert cpsievert deleted the feat/375-html-deps-rmarkdown branch May 15, 2023 19:40
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.

runtime: shiny in Rmarkdown doesn't know how to handle htmlDependency()
4 participants