Merge pull request #32 from rstudio/variable-overrides

Add .where and .default_flag args for better default behavior

Author: cpsievert

R/themes.R

@@ -16,6 +16,11 @@
 #' @param rules Any [sass::as_sass()] `input` to place after Bootstrap's SASS imports.
 #' @param ... For `bs_theme_add_variables()`, these arguments define SASS variables;
 #' otherwise, these arguments are passed along to [sass::sass_layer()].
+#' @param .where whether to place the variable definitions before other
+#' Sass `"defaults"`, after other Sass `"declarations"`, or after other Sass `"rules"`.
+#' @param .default_flag whether or not to add a `!default` flag (if missing) to
+#' variable expressions. It's recommended to keep this as `TRUE` when
+#' `.where = "defaults"`.
 #'
 #' @references \url{https://getbootstrap.com/docs/4.4/getting-started/theming/}
 #'
@@ -57,23 +62,55 @@ bs_theme_new <- function(version = version_default(), bootswatch = NULL) {
 
 #' @rdname theming
 #' @export
-bs_theme_add_variables <- function(...) {
+bs_theme_add_variables <- function(..., .where = "defaults",
+                                   .default_flag = identical(.where, "defaults")) {
   vars <- rlang::list2(...)
   if (any(names2(vars) == "")) stop("Variables must be named.", call. = FALSE)
-  bs_theme_add(vars)
+  .where <- match.arg(.where, c("defaults", "declarations", "rules"))
+
+  # Workaround to the problem of 'blue' winning in the scenario of:
+  # bs_theme_add_variables("body-bg" = "blue")
+  # bs_theme_add_variables("body-bg" = "red")
+  if (.default_flag) {
+    vars <- ensure_default_flag(vars)
+  }
+
+  do.call(bs_theme_add, setNames(list(vars), .where))
+}
+
+# Given a named list of variable definitions,
+# searches each variable's expression for a !default flag,
+# and if missing, adds it.
+ensure_default_flag <- function(vars) {
+  Map(
+    function(key, val) {
+      val <- paste(sass::as_sass(val), collapse = "\n")
+      if (grepl("!default\\s*;*\\s*$", val)) {
+        val
+      } else {
+        paste(sub(";+$", "", val), "!default")
+      }
+    }, names(vars), vars
+  )
 }
 
 #' @rdname theming
 #' @export
-bs_theme_add <- function(defaults = "", rules = "", ...) {
+bs_theme_add <- function(defaults = "", declarations = "", rules = "", ...) {
   old_theme <- bs_theme_get()
   if (is.null(old_theme)) {
     stop("Must call bs_theme_new() before adding to the theme.", call. = FALSE)
   }
-  layer <- if (inherits(defaults, "sass_layer")) {
-    defaults
+  if (inherits(defaults, "sass_layer")) {
+    if (!identical(declarations, "") || !identical(rules, "") || length(list(...)) > 0) {
+      stop(
+        "Not allowed to specify other arguments when the first argument, `defaults`, ",
+        "is a sass_layer()", call. = FALSE
+      )
+    }
+    layer <- defaults
   } else {
-    sass_layer(defaults = defaults, rules = rules, ...)
+    layer <- sass_layer(defaults = defaults, declarations = declarations, rules = rules, ...)
   }
   layer <- sass_layer_merge(old_theme, layer)
   bs_theme_set(add_class(layer, "bs_theme"))

tests/testthat/test-bs-theme.R

@@ -16,7 +16,7 @@ test_that("theme api works", {
   expect_equal(theme_bootswatch(cosmo_theme), "cosmo")
 
   # Can add new variable defaults
-  bs_theme_add_variables(primary = "red !default;")
+  bs_theme_add_variables("primary" = "red !default;")
   primary <- bootstrap_sass("body{background:$primary;}")
   expect_css("body{background:red;}", primary)
 
@@ -42,5 +42,20 @@ test_that("Theme adding works as intended", {
   bs_theme_add_variables(primary = "#fff !default;")
   css <- bootstrap_sass(".foo{color:$primary;}")
   expect_css(".foo{color:#fff;}", css)
+
+  # Also works without default flags and can handle numeric values
+  bs_theme_add_variables(primary = "blue")
+  bs_theme_add_variables(primary = "#fff", "font-size" = 0)
+  css <- bootstrap_sass(".foo{color:$primary;font-size:0}")
+  expect_css(".foo{color:#fff;font-size:0;}", css)
+
+  # Can also override variables via declarations
+  bs_theme_add_variables(
+    .where = "declarations",
+    "primary" = "$secondary",
+    "body-color" = "color-yiq($primary)"
+  )
+  css <- bootstrap_sass(".foo{color:$primary;}")
+  expect_css(".foo{color:#6c757d;}", css)
 })
 

tests/test-theme-bootswatch.R tests/testthat/test-theme-bootswatch.Rtests/test-theme-bootswatch.R renamed to tests/testthat/test-theme-bootswatch.R

@@ -2,10 +2,7 @@ context("theme_bootswatch")
 
 test_that("Can retrieve version from theme object", {
   bs_theme_new()
-  expect_identical(
-    theme_bootswatch(bs_theme_get()),
-    version_default()
-  )
+  expect_null(theme_bootswatch(bs_theme_get()))
 
   bs_theme_new(version = "3", bootswatch = "paper")
   expect_equal(