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

Some JS based features does not load correctly in Shiny runtime document #2032

Open
cpsievert opened this issue Feb 4, 2021 · 13 comments · May be fixed by #2046
Open

Some JS based features does not load correctly in Shiny runtime document #2032

cpsievert opened this issue Feb 4, 2021 · 13 comments · May be fixed by #2046
Labels
bug an unexpected problem or unintended behavior next to consider for next release theme: shiny regarding shiny support in R Markdown document

Comments

@cpsievert
Copy link
Contributor

For example

---
title: "My First R Markdown Document" 
runtime: shiny
output:
  html_document:
    anchor_sections: true
---

# R Markdown

## Overview

R Markdown combines markdown (an easy to write plain text format) with embedded
R code chunks.
@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

I found the issue is not limited to anchor_sections, but also to other scripts that listens to the DOMContentLoaded event.
Below is an example of header-attrs.js.
By inspecting the rendered page, section div and the corresponding heading has the same class "rmarkdown", which should be removed from the heading.

---
title: "My First R Markdown Document" 
runtime: shiny
output: html_document
---

# R Markdown {.rmarkdown}

## Overview

R Markdown combines markdown (an easy to write plain text format) with embedded
R code chunks.

@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

I also think KaTeX with runtime:shiny in my PR does not work due to this issue.

#1940 (comment)

@cderv cderv added the bug an unexpected problem or unintended behavior label Feb 5, 2021
@cderv
Copy link
Collaborator

cderv commented Feb 5, 2021

Thanks @cpsievert for the report and thanks @atusy for the start of investigation.

We'll need to have a deep look into that - Rmd with shiny runtime is not always easy to debug.

@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

By DevTools, we can see the real R Markdown content is inserted by the iframe element.
I think the insertion takes place afterDOMContentLoaded, and that is why event handlers do not fire if they listening to DOMContentLoaded.
In some case, we can workaround the issue by using async or defer.

Reprex

---
title: "My First R Markdown Document" 
output: html_document
runtime: shiny
---

```{=html}
<script>
document.addEventListener("DOMContentLoaded", function() {
console.log("DOMContentLoaded")
})
</script>

<script async>
console.log("async")
</script>

<script defer>
console.log("defer")
</script>
```

@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

Just to note I opened a PR rstudio/htmltools#201 so that we can easily async/defer scripts.

@cderv
Copy link
Collaborator

cderv commented Feb 5, 2021

So this means that any html dependencies used in R Markdown and using a listener on DOMContentLoaded won't be effective when using runtime shiny - Is that so ?

@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

Yes... as far as I understand.

@cpsievert
Copy link
Contributor Author

cpsievert commented Feb 5, 2021

Ok, I merged your PR in htmltools (we were already planning on making that fix).

It might also be worth looking into just using $(function() {}) instead of the defer+DOMContentLoaded approach since that'd be more consistent with how do things elsewhere in Shiny & R Markdown

@atusy
Copy link
Contributor

atusy commented Feb 5, 2021

@cpsievert Thank you so much. I agree $(function() {}) is the robust way, however, it means anchor_sections and other scripts require jQuery. There are cases where people want to use html_document without jQuery (i.e., theme = NULL). I think it is nice if we can provide the functionalities without jQuery as much as possible.

@cderv
Copy link
Collaborator

cderv commented Feb 5, 2021

@atusy was quicker than me 😄

Yes JQuery syntax is definitely an option. I am always trying to not depend on JQuery for new (small) scripts in order to maybe one day, have JQuery-free document. This is a wish from some member of the community for the R Markdown output produced. And indeed already possible with theme = NULL.

In shiny ecosystem, will you keep rely on JQuery ? I understand bootstrap 5 will not use it (by default).

@cpsievert
Copy link
Contributor Author

In shiny ecosystem, will you keep rely on JQuery?

I don't see it going away anytime soon

@cderv
Copy link
Collaborator

cderv commented Feb 5, 2021

Ok thanks.

For R Markdown, currently JQuery dependency is only inserted with theme != NULL. So it is a matter to decide with features powered by html dependencies with JS should be available for custom theming. (that advanced users are using from what I know)

@atusy atusy linked a pull request Feb 14, 2021 that will close this issue
@atusy
Copy link
Contributor

atusy commented Feb 14, 2021

I re-implemented dependencies with the defer attribute, however, there needs some more improvements to support runtime: shiny.
See #2046 for the details.

@cderv cderv added the theme: shiny regarding shiny support in R Markdown document label Jan 6, 2022
@cderv cderv changed the title anchor_sections: true doesn't appear to work with runtime: shiny Some JS based features does not load correctly in Shiny runtime document Jan 6, 2022
@cderv cderv added the next to consider for next release label Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior next to consider for next release theme: shiny regarding shiny support in R Markdown document
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants