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 invalid HTML code in shiny_prerendered documents #1942

Merged
merged 6 commits into from
Feb 25, 2021

Conversation

dakep
Copy link
Contributor

@dakep dakep commented Nov 6, 2020

Resolves #1912. Instead of an HTML string containing the entire document (which is not recognized as stand-alone html document by shiny), return an html_document object with the dependencies already injected.

@CLAassistant
Copy link

CLAassistant commented Nov 6, 2020

CLA assistant check
All committers have signed the CLA.

@ghost
Copy link

ghost commented Feb 23, 2021

Thanks a ton. By when this will be merged with rstudio:master?

Copy link
Contributor

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

I'd like to allow for htmltools to still have control over the HEAD_CONTENT comment that it looks for. We can use {{ headContent() }} instead to allow htmltools to change in the future (unlikely).

LGTM!(... given small change). Thank you for the changes!

R/shiny_prerendered.R Outdated Show resolved Hide resolved
R/shiny_prerendered.R Outdated Show resolved Hide resolved

# create shiny app
shiny::shinyApp(
ui = function(req) html,
ui = function(req) html_doc,
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this change, but this can just be ui = html_doc, since the value is static

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jcheng5 it seems it has been made a function to avoid caching of html_prerendered UI. See
2de1b1d

Has it changed ? Should it be kept ?

Copy link
Collaborator

@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.

Thanks @jcheng5 and @schloerke for the review.

@dakep I'll do the change and merge. Thanks a lot for this PR !

@cderv cderv merged commit 49c0c2b into rstudio:master Feb 25, 2021
@cderv
Copy link
Collaborator

cderv commented Feb 25, 2021

Thanks @dakep !

jonathan-g added a commit to jonathan-g/rmarkdown that referenced this pull request Mar 3, 2021
Merge remote-tracking branch 'rstudio_origin/master' into jg-devel

* rstudio_origin/master: (26 commits)
  Use data-external=1 to prevent encoding by Pandoc of img src
  add `subdirs` documentation for site generator (rstudio#2058)
  fix: corrected typo in `ghostscript` (rstudio#2059)
  Add CoC contact email
  quillt has moved to rstudio/quillt
  fix invalid HTML code in shiny_prerendered documents (rstudio#1942)
  Allow pkgdown PR preview (rstudio#2055)
  Adding pkgdown site built by GHA (rstudio#1955)
  Add shiny bslib dependency inside html_document_base pre-process (rstudio#2049)
  start the next version
  CRAN release v2.7
  place <div> in \code{}
  amend 62ae2ed and rstudio#1989: make sure output_format_filter is a function; testing NULL is not enough, e.g., the radix package still calls
  use CRAN releases of shiny and bslib
  tweak news
  Only keep the first url in citation  (rstudio#2041)
  Fix for knit_print.data.frame (rstudio#2050)
  set gfm_auto_identifiers to input format if toc is TRUE (rstudio#2040)
  Enable pkgdown/downlit cross-package links to rmarkdown (rstudio#1843)
  update CONTRIBUTING.md
  ...

# Conflicts:
#	DESCRIPTION
cderv added a commit that referenced this pull request Mar 4, 2021
A new pandoc variable `shiny-prerendered` is set when rendering a shiny prerendered document. This allow to add specific content for this type of rendered document in an HTML template. 
It is used in all HTML templates to add a special comment `<!-- HEAD_CONTENT -->` required by htmltools / shiny to reinsert the HTML dependencies. Following #1942, it was put by default at the end of the error which caused issue in leanr, e.g rstudio/learnr#499

This will fix #2060 following change in 49c0c2b.
cderv added a commit to rstudio/flexdashboard that referenced this pull request Jun 22, 2021
cpsievert pushed a commit to rstudio/flexdashboard that referenced this pull request Jun 23, 2021
* Add HEAD placeholder in html template for shiny_prerendered

This follows change in rstudio/rmarkdown#1942 and rstudio/rmarkdown#2064

* update NEWS
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid HTML in prerendered shiny documents
6 participants