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

Add shiny::bootstrapLib() as a dependency inside html_document_base()'s pre-processor #2049

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

cpsievert
Copy link
Contributor

@cpsievert cpsievert commented Feb 18, 2021

In #1706, to make bslib::bs_themer() work with runtime: shiny I added shiny::bootstrapLib() as a dependency when run() is first called. This ends up being is too early for output formats that wish to pass a modified theme to html_document_base() (the new version of flexdashboard::flex_dashboard will do this).

If you care for an example, first install rstudio/flexdashboard#308, then render a flexdashboard example. Note that with this PR will make it so the navbar's bg color reflects $primary (instead of $dark)

…l shiny::bootstrapLib() inside html_document_base()'s preprocessor (instead of at run()-time)

The former time-point is too early -- it doesn't allow other output formats like flex_dashboard to modify the theme before passing it to html_document_base()
@cpsievert
Copy link
Contributor Author

cpsievert commented Feb 19, 2021

Ahh, in order for runtime: shiny_prerendered to work with bslib::bs_themer(), we also need to call shiny:::setCurrentTheme() so that the server-side code can know about the document's theme. Currently that function isn't exported in shiny since we haven't found a real use-case for it outside of shiny, but I can try to convince Winston to export it :)

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

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

@cderv Usually I don't review PRs related to shiny and would just merge them as is. If you want to take a closer look, please feel free to, otherwise I'm okay if you just merge it. Thanks!

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.

LGTM !

Thanks for your patience on this one @cpsievert and thanks for the work on bslib in rmarkdown !!

@cderv cderv merged commit e455285 into rstudio:master Feb 22, 2021
@cpsievert cpsievert deleted the bslib-shiny-runtime branch February 22, 2021 16:19
sthagen added a commit to sthagen/rstudio-rmarkdown that referenced this pull request Feb 23, 2021
Add shiny bslib dependency inside html_document_base pre-process (rstudio#2049)
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
@@ -134,16 +134,21 @@ run <- function(file = "index.Rmd", dir = dirname(file), default_file = NULL,
yaml_front <- if (length(target_file)) yaml_front_matter(target_file)
runtime <- yaml_front$runtime
theme <- render_args$output_options$theme
Copy link
Collaborator

@cderv cderv Mar 3, 2021

Choose a reason for hiding this comment

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

@cpsievert as I am going through shiny prerendered code base to understand better how it works, I am just wondering what this line is used for ? It does not seem to be used now with the PR changes.

Do you recall the aim of catching a special handling when theme would be passed inside rmarkdown::run(render_args=) ?

R/shiny.R Show resolved Hide resolved
@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.

3 participants