Merge branch 'main' into fix/error-console-dedupe-messages

DESCRIPTION

@@ -85,7 +85,7 @@ Imports:
     later (>= 1.0.0),
     promises (>= 1.3.2),
     tools,
-    crayon,
+    cli,
     rlang (>= 0.4.10),
     fastmap (>= 1.1.1),
     withr,
@@ -99,7 +99,7 @@ Suggests:
     datasets,
     DT,
     Cairo (>= 1.5-5),
-    testthat (>= 3.0.0),
+    testthat (>= 3.2.1),
     knitr (>= 1.6),
     markdown,
     rmarkdown,
@@ -210,7 +210,6 @@ Collate:
 RoxygenNote: 7.3.2
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RdMacros: lifecycle
 Config/testthat/edition: 3
 Config/Needs/check:
   shinytest2

NEWS.md

@@ -1,5 +1,14 @@
 # shiny (development version)
 
+## New features and improvements
+
+* When busy indicators are enabled (i.e., `useBusyIndicators()`), Shiny now:
+    * Shows a spinner on recalculating htmlwidgets that have previously rendered an error (including `req()` and `validate()`). (#4172)
+    * Shows a spinner on `tableOutput()`. (#4172)
+    * Places a minimum height on recalculating outputs so that the spinner is always visible. (#4172)
+
+* Shiny now uses `{cli}` instead of `{crayon}` for rich log messages. (@olivroy #4170)
+
 ## Bug fixes
 
 * Fixed a bug with modals where calling `removeModal()` too quickly after `showModal()` would fail to remove the modal if the remove modal message was received while the modal was in the process of being revealed. (#4173)

R/bootstrap.R

@@ -1113,7 +1113,7 @@ plotOutput <- function(outputId, width = "100%", height="400px",
 #' @rdname renderTable
 #' @export
 tableOutput <- function(outputId) {
-  div(id = outputId, class="shiny-html-output")
+  div(id = outputId, class="shiny-html-output shiny-table-output")
 }
 
 dataTableDependency <- list(

R/conditions.R

@@ -417,11 +417,11 @@ printOneStackTrace <- function(stackTrace, stripResult, full, offset) {
       ": ",
       mapply(paste0(st$call, st$loc), st$category, FUN = function(name, category) {
         if (category == "pkg")
-          crayon::silver(name)
+          cli::col_silver(name)
         else if (category == "user")
-          crayon::blue$bold(name)
+          cli::style_bold(cli::col_blue(name))
         else
-          crayon::white(name)
+          cli::col_white(name)
       }),
       "\n"
     )

R/test.R

@@ -158,8 +158,7 @@ print.shiny_runtests <- function(x, ..., reporter = "summary") {
 
 
   if (any(x$pass)) {
-    # TODO in future... use clisymbols::symbol$tick and crayon green
-    cat("* Success\n")
+    cli::cat_bullet("Success", bullet = "tick", bullet_col = "green")
     mapply(
       x$file,
       x$pass,
@@ -171,9 +170,8 @@ print.shiny_runtests <- function(x, ..., reporter = "summary") {
       }
     )
   }
-  if (any(!x$pass)) {
-    # TODO in future... use clisymbols::symbol$cross and crayon red
-    cat("* Failure\n")
+  if (!all(x$pass)) {
+    cli::cat_bullet("Failure", bullet = "cross", bullet_col = "red")
     mapply(
       x$file,
       x$pass,

srcts/extras/busy-indicators/busy-indicators.scss

@@ -7,6 +7,8 @@
 
   .recalculating {
 
+    min-height: var(--shiny-spinner-size, 32px);
+
     &::after {
       position: absolute;
       content: "";
@@ -45,13 +47,31 @@
       transition: opacity 250ms ease var(--shiny-spinner-delay, 1s);
     }
 
+    /*
+      When htmlwidget errors are rendered, an inline `visibility:hidden` is put
+      on the html-widget-output, and the error message (if any) is put in a
+      sibling element that overlays the output container (this way, the height
+      of the output container doesn't change). Work around this by making the
+      output container itself visible and making the children (except the
+      spinner) invisible.
+    */
+    &.html-widget-output {
+      visibility: inherit !important;
+      > * {
+        visibility: hidden;
+      }
+      ::after {
+        visibility: visible;
+      }
+    }
+
     /*
       Disable spinner on uiOutput() mainly because (for other reasons) it has
       `display:contents`, which breaks the ::after positioning.
       Note that, even if we could position it, we'd probably want to disable it
       if it has recalculating children.
     */
-    &.shiny-html-output::after {
+    &.shiny-html-output:not(.shiny-table-output)::after {
       display: none;
     }
   }
@@ -105,6 +125,9 @@
   &.shiny-busy:has(.recalculating:not(.shiny-html-output))::after {
     display: none;
   }
+  &.shiny-busy:has(.recalculating.shiny-table-output)::after {
+    display: none;
+  }
   &.shiny-busy:has(#shiny-disconnected-overlay)::after {
     display: none;
   }

tests/testthat/test-bootstrap.r

@@ -21,10 +21,11 @@ test_that("Repeated names for selectInput and radioButtons choices", {
 
   # Select input
   x <- selectInput('id','label', choices = c(a='x1', a='x2', b='x3'), selectize = FALSE)
-  expect_true(grepl(fixed = TRUE,
+  expect_match(
+    format(x),
     '<select class="shiny-input-select form-control" id="id"><option value="x1" selected>a</option>\n<option value="x2">a</option>\n<option value="x3">b</option></select>',
-     format(x)
-  ))
+    fixed = TRUE
+  )
 
   # Radio buttons using choices
   x <- radioButtons('id','label', choices = c(a='x1', a='x2', b='x3'))
@@ -248,10 +249,11 @@ test_that("selectInput selects items by default", {
   ))
 
   # Nothing selected when choices=NULL
-  expect_true(grepl(fixed = TRUE,
+  expect_match(
+    format(selectInput('x', NULL, NULL, selectize = FALSE)),
     '<select class="shiny-input-select form-control" id="x"></select>',
-    format(selectInput('x', NULL, NULL, selectize = FALSE))
-  ))
+    fixed = TRUE
+  )
 
   # None specified as selected. With multiple=TRUE, none selected by default.
   expect_true(grepl(fixed = TRUE,

tests/testthat/test-busy-indication.R

@@ -48,7 +48,7 @@ test_that("busyIndicatorOptions()", {
 
 
 test_that("Can provide svg file for busyIndicatorOptions(spinner_type)", {
-  skip_if(.Platform$OS.type == "windows")
+  skip_on_os("windows")
 
   tmpsvg <- tempfile(fileext = ".svg")
   writeLines("<svg></svg>", tmpsvg)

tests/testthat/test-input-select.R

@@ -1,10 +1,10 @@
 test_that("performance warning works", {
   pattern <- "consider using server-side selectize"
 
-  expect_warning(selectInput("x", "x", as.character(1:999)), NA)
-  expect_warning(selectInput("x", "x", as.character(1:999), selectize = TRUE), NA)
-  expect_warning(selectInput("x", "x", as.character(1:999), selectize = FALSE), NA)
-  expect_warning(selectizeInput("x", "x", as.character(1:999)), NA)
+  expect_no_warning(selectInput("x", "x", as.character(1:999)))
+  expect_no_warning(selectInput("x", "x", as.character(1:999), selectize = TRUE))
+  expect_no_warning(selectInput("x", "x", as.character(1:999), selectize = FALSE))
+  expect_no_warning(selectizeInput("x", "x", as.character(1:999)))
 
   expect_warning(selectInput("x", "x", as.character(1:1000)), pattern)
   expect_warning(selectInput("x", "x", as.character(1:1000), selectize = TRUE), pattern)
@@ -17,9 +17,9 @@ test_that("performance warning works", {
 
   session <- MockShinySession$new()
 
-  expect_warning(updateSelectInput(session, "x", choices = as.character(1:999)), NA)
-  expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:999)), NA)
-  expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:999), server = FALSE), NA)
+  expect_no_warning(updateSelectInput(session, "x", choices = as.character(1:999)))
+  expect_no_warning(updateSelectizeInput(session, "x", choices = as.character(1:999)))
+  expect_no_warning(updateSelectizeInput(session, "x", choices = as.character(1:999), server = FALSE))
 
   expect_warning(updateSelectInput(session, "x", choices = as.character(1:1000)), pattern)
   expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:1000)), pattern)
@@ -28,9 +28,9 @@ test_that("performance warning works", {
   expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:2000)), pattern)
   expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:2000), server = FALSE), pattern)
 
-  expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:999), server = TRUE), NA)
-  expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:1000), server = TRUE), NA)
-  expect_warning(updateSelectizeInput(session, "x", choices = as.character(1:2000), server = TRUE), NA)
+  expect_no_warning(updateSelectizeInput(session, "x", choices = as.character(1:999), server = TRUE))
+  expect_no_warning(updateSelectizeInput(session, "x", choices = as.character(1:1000), server = TRUE))
+  expect_no_warning(updateSelectizeInput(session, "x", choices = as.character(1:2000), server = TRUE))
 })
 
 
@@ -55,9 +55,9 @@ test_that("selectInput options are properly escaped", {
   ))
 
   si_str <- as.character(si)
-  expect_true(any(grepl("<option value=\"&quot;\">", si_str, fixed = TRUE)))
-  expect_true(any(grepl("<option value=\"&#39;\">", si_str, fixed = TRUE)))
-  expect_true(any(grepl("<optgroup label=\"&quot;Separators&quot;\">", si_str, fixed = TRUE)))
+  expect_match(si_str, "<option value=\"&quot;\">", fixed = TRUE, all = FALSE)
+  expect_match(si_str, "<option value=\"&#39;\">", fixed = TRUE, all = FALSE)
+  expect_match(si_str, "<optgroup label=\"&quot;Separators&quot;\">", fixed = TRUE, all = FALSE)
 })
 
 
@@ -75,10 +75,10 @@ test_that("selectInputUI has a select at an expected location", {
         )
         # if this getter is changed, varSelectInput getter needs to be changed
         selectHtml <- selectInputVal$children[[2]]$children[[1]]
-        expect_true(inherits(selectHtml, "shiny.tag"))
+        expect_s3_class(selectHtml, "shiny.tag")
         expect_equal(selectHtml$name, "select")
         if (!is.null(selectHtml$attribs$class)) {
-          expect_false(grepl(selectHtml$attribs$class, "symbol"))
+          expect_no_match(selectHtml$attribs$class, "symbol")
         }
 
         varSelectInputVal <- varSelectInput(
@@ -91,9 +91,9 @@ test_that("selectInputUI has a select at an expected location", {
         )
         # if this getter is changed, varSelectInput getter needs to be changed
         varSelectHtml <- varSelectInputVal$children[[2]]$children[[1]]
-        expect_true(inherits(varSelectHtml, "shiny.tag"))
+        expect_s3_class(varSelectHtml, "shiny.tag")
         expect_equal(varSelectHtml$name, "select")
-        expect_true(grepl("symbol", varSelectHtml$attribs$class, fixed = TRUE))
+        expect_match(varSelectHtml$attribs$class, "symbol", fixed = TRUE)
       }
     }
   }

tests/testthat/test-plot-png.R

@@ -2,5 +2,5 @@ test_that("plotPNG()/startPNG() ignores NULL dimensions", {
   f <- plotPNG(function() plot(1), width = NULL, height = NULL)
   on.exit(unlink(f))
   bits <- readBin(f, "raw", file.info(f)$size)
-  expect_true(length(bits) > 0)
+  expect_gt(length(bits), 0)
 })

tests/testthat/test-reactivity.r

@@ -9,7 +9,7 @@ test_that("ReactiveVal", {
   val <- reactiveVal()
 
   isolate({
-    expect_true(is.null(val()))
+    expect_null(val())
 
     # Set to a simple value
     val(1)
@@ -99,12 +99,12 @@ test_that("ReactiveValues", {
   values <- reactiveValues(a=NULL, b=2)
   # a should exist and be NULL
   expect_setequal(isolate(names(values)), c("a", "b"))
-  expect_true(is.null(isolate(values$a)))
+  expect_null(isolate(values$a))
 
   # Assigning NULL should keep object (not delete it), and set value to NULL
   values$b <- NULL
   expect_setequal(isolate(names(values)), c("a", "b"))
-  expect_true(is.null(isolate(values$b)))
+  expect_null(isolate(values$b))
 
 
   # Errors -----------------------------------------------------------------
@@ -960,8 +960,8 @@ test_that("classes of reactive object", {
 })
 
 test_that("{} and NULL also work in reactive()", {
-  expect_error(reactive({}), NA)
-  expect_error(reactive(NULL), NA)
+  expect_no_error(reactive({}))
+  expect_no_error(reactive(NULL))
 })
 
 test_that("shiny.suppressMissingContextError option works", {

tests/testthat/test-render-functions.R

@@ -29,8 +29,8 @@ test_that("Render functions correctly handle quosures", {
   r1 <- inject(renderTable({ pressure[!!a, ] }, digits = 1))
   r2 <- renderTable({ eval_tidy(quo(pressure[!!a, ])) }, digits = 1)
   a <- 2
-  expect_true(grepl("0\\.0", r1()))
-  expect_true(grepl("20\\.0", r2()))
+  expect_match(r1(), "0\\.0")
+  expect_match(r2(), "20\\.0")
 })
 
 test_that("functionLabel returns static value when the label can not be assigned to", {

tests/testthat/test-stacks.R

@@ -227,7 +227,7 @@ test_that("observeEvent is not overly stripped (#4162)", {
     })
   )
   st_str <- capture.output(printStackTrace(caught), type = "message")
-  expect_true(any(grepl("observeEvent\\(1\\)", st_str)))
+  expect_match(st_str, "observeEvent\\(1\\)", all = FALSE)
 
   # Now same thing, but deep stack trace version
 
@@ -257,6 +257,6 @@ test_that("observeEvent is not overly stripped (#4162)", {
   )
   st_str <- capture.output(printStackTrace(caught), type = "message")
   # cat(st_str, sep = "\n")
-  expect_true(any(grepl("A__", st_str)))
-  expect_true(any(grepl("B__", st_str)))
+  expect_match(st_str, "A__", all = FALSE)
+  expect_match(st_str, "B__", all = FALSE)
 })

tests/testthat/test-tabPanel.R

@@ -115,7 +115,5 @@ test_that("tabItem titles can contain tag objects", {
   #   "<a ....> <i>Hello</i> world"
   # As opposed to:
   #   "<a ....>&lt;i&gt;Hello&lt;/i&gt; world
-  expect_true(
-    grepl("<a [^>]+>\\s*<i>Hello</i>\\s+world", x$html)
-  )
+  expect_match(x$html, "<a [^>]+>\\s*<i>Hello</i>\\s+world")
 })

tests/testthat/test-test-runTests.R

@@ -122,7 +122,7 @@ test_that("runTests runs as expected without rewiring", {
   appDir <- test_path(file.path("..", "test-helpers", "app1-standard"))
   df <- testthat::expect_output(
     print(runTests(appDir = appDir, assert = FALSE)),
-    "Shiny App Test Results\\n\\* Success\\n  - app1-standard/tests/runner1\\.R\\n  - app1-standard/tests/runner2\\.R"
+    "Shiny App Test Results\\n\\v Success\\n  - app1-standard/tests/runner1\\.R\\n  - app1-standard/tests/runner2\\.R"
   )
 
   expect_equal(df, data.frame(