From 6cd389ab207255bd6f0de4e6277e22060f994bb9 Mon Sep 17 00:00:00 2001 From: christophe dervieux Date: Wed, 3 Mar 2021 17:03:37 +0100 Subject: [PATCH 1/3] render_args passed to run can contain theme and should be also consider for bslib theme resolution in shiny --- R/shiny.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/shiny.R b/R/shiny.R index b666963e0e..28c5bc681a 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -133,14 +133,16 @@ run <- function(file = "index.Rmd", dir = dirname(file), default_file = NULL, target_file <- file %||% file.path(dir, default_file) yaml_front <- if (length(target_file)) yaml_front_matter(target_file) runtime <- yaml_front$runtime - theme <- render_args$output_options$theme # Let shiny::getCurrentTheme() know about the yaml's theme, so # things like `bslib::bs_themer()` can work with prerendered documents. # Also note that we add the actual shiny::bootstrapLib() dependency # inside the document's pre-processing hook so the 'last' version of # the theme wins out if (length(target_file)) { - format <- output_format_from_yaml_front_matter(read_utf8(target_file)) + format <- output_format_from_yaml_front_matter( + input_lines = read_utf8(target_file), + output_options = render_args$output_options + ) set_current_theme(resolve_theme(format$options$theme)) } From ff6c7b85bc4be13be411de2ce076678ef1e0575d Mon Sep 17 00:00:00 2001 From: christophe dervieux Date: Thu, 4 Mar 2021 12:57:25 +0100 Subject: [PATCH 2/3] Pass only bs_theme object to shiny setCurrentTheme --- R/html_dependencies.R | 3 +-- R/shiny.R | 2 +- tests/testthat/test-shiny.R | 27 ++++++++++++++++++++------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/R/html_dependencies.R b/R/html_dependencies.R index 99280d6463..8670b9b84b 100644 --- a/R/html_dependencies.R +++ b/R/html_dependencies.R @@ -126,8 +126,7 @@ as_bs_theme <- function(theme) { } is_bs_theme <- function(theme) { - is_available("bslib") && - bslib::is_bs_theme(theme) + is_available("bslib") && bslib::is_bs_theme(theme) } theme_version <- function(theme) { diff --git a/R/shiny.R b/R/shiny.R index 28c5bc681a..d51168d904 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -597,5 +597,5 @@ read_shiny_deps <- function(files_dir) { # shiny:::setCurrentTheme() was added in 1.6 (we may export in next version) set_current_theme <- function(theme) { set_theme <- asNamespace("shiny")$setCurrentTheme - if (is.function(set_theme)) set_theme(theme) + if (is.function(set_theme) && is_bs_theme(theme)) set_theme(theme) } diff --git a/tests/testthat/test-shiny.R b/tests/testthat/test-shiny.R index c4afa14910..8b5b861851 100644 --- a/tests/testthat/test-shiny.R +++ b/tests/testthat/test-shiny.R @@ -18,13 +18,26 @@ test_that("file.path.ci returns correctly no matter the case", { }) -test_that("set_current_theme() informs shiny::getCurrentTheme()", { - expect_null(shiny::getCurrentTheme()) - theme <- bslib::bs_theme() - set_current_theme(theme) - expect_equal(theme, shiny::getCurrentTheme()) - set_current_theme(NULL) - expect_null(shiny::getCurrentTheme()) +test_that("set_current_theme() informs shiny::getCurrentTheme() only with bslib theme", { + skip_if_not(packageVersion("shiny") >= 1.6) + with_clean_shinyTheme <- function(expr) { + shiny:::setCurrentTheme(NULL) + force(expr) + shiny:::setCurrentTheme(NULL) + } + with_clean_shinyTheme({ + theme <- bslib::bs_theme() + set_current_theme(theme) + expect_equal(theme, shiny::getCurrentTheme()) + }) + with_clean_shinyTheme({ + set_current_theme(NULL) + expect_null(shiny::getCurrentTheme()) + }) + with_clean_shinyTheme({ + set_current_theme("cerulean") + expect_null(shiny::getCurrentTheme()) + }) }) test_that("html_prerendered is a full document template to use as UI for shiny", { From 3ede92fdbdd48d724475bb5aab5d7ef827c9ba7d Mon Sep 17 00:00:00 2001 From: christophe dervieux Date: Thu, 4 Mar 2021 22:10:29 +0100 Subject: [PATCH 3/3] theme is not necessary a bs_theme object revert previous changes ff6c7b85bc4be13be411de2ce076678ef1e0575d --- R/shiny.R | 2 +- tests/testthat/test-shiny.R | 26 ++++++++------------------ 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/R/shiny.R b/R/shiny.R index d51168d904..28c5bc681a 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -597,5 +597,5 @@ read_shiny_deps <- function(files_dir) { # shiny:::setCurrentTheme() was added in 1.6 (we may export in next version) set_current_theme <- function(theme) { set_theme <- asNamespace("shiny")$setCurrentTheme - if (is.function(set_theme) && is_bs_theme(theme)) set_theme(theme) + if (is.function(set_theme)) set_theme(theme) } diff --git a/tests/testthat/test-shiny.R b/tests/testthat/test-shiny.R index 8b5b861851..37dc1147a4 100644 --- a/tests/testthat/test-shiny.R +++ b/tests/testthat/test-shiny.R @@ -20,24 +20,14 @@ test_that("file.path.ci returns correctly no matter the case", { test_that("set_current_theme() informs shiny::getCurrentTheme() only with bslib theme", { skip_if_not(packageVersion("shiny") >= 1.6) - with_clean_shinyTheme <- function(expr) { - shiny:::setCurrentTheme(NULL) - force(expr) - shiny:::setCurrentTheme(NULL) - } - with_clean_shinyTheme({ - theme <- bslib::bs_theme() - set_current_theme(theme) - expect_equal(theme, shiny::getCurrentTheme()) - }) - with_clean_shinyTheme({ - set_current_theme(NULL) - expect_null(shiny::getCurrentTheme()) - }) - with_clean_shinyTheme({ - set_current_theme("cerulean") - expect_null(shiny::getCurrentTheme()) - }) + expect_null(shiny::getCurrentTheme()) + set_current_theme(theme <- bslib::bs_theme()) + expect_equal(theme, shiny::getCurrentTheme()) + set_current_theme(NULL) + expect_null(shiny::getCurrentTheme()) + set_current_theme(theme <- "cerulean") + expect_equal(theme, shiny::getCurrentTheme()) + set_current_theme(NULL) }) test_that("html_prerendered is a full document template to use as UI for shiny", {