Skip to content

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.

@gadenbuie gadenbuie requested review from cderv and schloerke April 19, 2023 15:46
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
@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()

5 participants