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

bootstrapLib() now always sets state on render and cleans up post static render #3402

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: shiny
Type: Package
Title: Web Application Framework for R
Version: 1.6.0.9021
Version: 1.6.0.9022
Authors@R: c(
person("Winston", "Chang", role = c("aut", "cre"), email = "[email protected]", comment = c(ORCID = "0000-0002-1576-2126")),
person("Joe", "Cheng", role = "aut", email = "[email protected]"),
Expand Down Expand Up @@ -79,7 +79,7 @@ Imports:
jsonlite (>= 0.9.16),
xtable,
fontawesome (>= 0.2.1),
htmltools (>= 0.5.1.9003),
htmltools (>= 0.5.1.9006),
R6 (>= 2.0),
sourcetools,
later (>= 1.0.0),
Expand Down
69 changes: 23 additions & 46 deletions R/bootstrap.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ bootstrapPage <- function(..., title = NULL, theme = NULL, lang = NULL) {
# the tagList() contents to avoid breaking user code that makes assumptions
# about the return value https://github.com/rstudio/shiny/issues/3235
if (is_bs_theme(theme)) {
args <- c(bootstrapLib(theme), args)
args <- list(bootstrapLib(theme), args)
ui <- do.call(tagList, args)
} else {
ui <- do.call(tagList, args)
Expand Down Expand Up @@ -82,53 +82,30 @@ getLang <- function(ui) {
#' @inheritParams bootstrapPage
#' @export
bootstrapLib <- function(theme = NULL) {
tagFunction(function() {
if (isRunning()) {
# In the non-bslib case, return static HTML dependencies
if (!is_bs_theme(theme)) {
return(bootstrapDependency(theme))
}
# To support static rendering of Bootstrap version dependent markup (e.g.,
# tabsetPanel()), setCurrentTheme() at the start of the render (since
# bootstrapLib() comes first in bootstrapPage(), all other UI should then know
# what version of Bootstrap is being used). Then restore state after render as
# long as shiny isn't running (in that case, since setCurrentTheme() uses
# shinyOptions(), state will be automatically be restored when the app exits)
oldTheme <- NULL
tagList(
.renderHook = function(x) {
oldTheme <<- getCurrentTheme()
setCurrentTheme(theme)
# For refreshing Bootstrap CSS when session$setCurrentTheme() happens
if (isRunning()) registerThemeDependency(bs_theme_deps)
attachDependencies(x, bslib::bs_theme_dependencies(theme))
},
.postRenderHook = function() {
if (!isRunning()) setCurrentTheme(oldTheme)
NULL
}

# If we're not compiling Bootstrap Sass (from bslib), return the
# static Bootstrap build.
if (!is_bs_theme(theme)) {
# We'll enter here if `theme` is the path to a .css file, like that
# provided by `shinythemes::shinytheme("darkly")`.
return(bootstrapDependency(theme))
}

# Make bootstrap Sass available so other tagFunction()s (e.g.,
# sliderInput() et al) can resolve their HTML dependencies at render time
# using getCurrentTheme(). Note that we're making an implicit assumption
# that this tagFunction() executes *before* all other tagFunction()s; but
# that should be fine considering that, DOM tree order is preorder,
# depth-first traversal, and at least in the bootstrapPage(theme) case, we
# have control over the relative ordering.
# https://dom.spec.whatwg.org/#concept-tree
# https://stackoverflow.com/a/16113998/1583084
#
# Note also that since this is shinyOptions() (and not options()), the
# option is automatically reset when the app (or session) exits
if (isRunning()) {
registerThemeDependency(bs_theme_deps)

} else {
# Technically, this a potential issue (someone trying to execute/render
# bootstrapLib outside of a Shiny app), but it seems that, in that case,
# you likely have other problems, since sliderInput() et al. already assume
# that Shiny is the one doing the rendering
#warning(
# "It appears `shiny::bootstrapLib()` was rendered outside of an Shiny ",
# "application context, likely by calling `as.tags()`, `as.character()`, ",
# "or `print()` directly on `bootstrapLib()` or UI components that may ",
# "depend on it (e.g., `fluidPage()`, etc). For 'themable' UI components ",
# "(e.g., `sliderInput()`, `selectInput()`, `dateInput()`, etc) to style ",
# "themselves based on the Bootstrap theme, make sure `bootstrapLib()` is ",
# "provided directly to the UI and that the UI is provided direction to ",
# "`shinyApp()` (or `runApp()`)", call. = FALSE
#)
}

bslib::bs_theme_dependencies(theme)
})
)
}

# This is defined outside of bootstrapLib() because registerThemeDependency()
Expand Down