tests: add unit and shinytest2 tests for downloadButton/Link enabled param

- Add missing `downloadLink starts disabled with correct attributes` unit test - Add shinytest2 runtime tests covering all three `enabled` values for both downloadButton and downloadLink: initial state (auto-enable, stays disabled, starts enabled, shinyjs-disabled) and toggle on/off via a setEnabled custom message handler that mirrors shinyjs::enable()/disable() - Clarify `enabled=FALSE` docs: the opt-out applies for the page lifetime via data-ignore-update, so renderValue never auto-enables regardless of runtime state - Remove unreachable dead code in DownloadLinkOutputBinding.showProgress() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-29 03:00:45 -04:00 · 2026-04-23 17:38:04 -04:00
parent 958027cdfa
commit bc34f3443f
12 changed files with 391 additions and 34 deletions
--- a/R/bootstrap.R
+++ b/R/bootstrap.R
@@ -1242,9 +1242,10 @@ uiOutput <- htmlOutput
 #'     enabled when the `downloadHandler` provides a non-`NULL` filename.
 #'   - `TRUE`: the button starts enabled immediately, without waiting for the
 #'     `downloadHandler`.
-#'   - `FALSE`: the button remains disabled and Shiny will not automatically
-#'     enable it; you must manage the enabled/disabled state yourself (e.g.,
-#'     with [shinyjs::enable()] and [shinyjs::disable()]).
+#'   - `FALSE`: the button starts disabled and Shiny will **never**
+#'     automatically enable it, even after the `downloadHandler` is ready.
+#'     You are responsible for managing the enabled/disabled state yourself
+#'     (e.g., with [shinyjs::enable()] and [shinyjs::disable()]).
 #' @param ... Other arguments to pass to the container tag function.
 #'
 #' @examples
@@ -1280,8 +1281,8 @@ uiOutput <- htmlOutput
 #' @seealso [downloadHandler()]
 #' @export
 downloadButton <- function(outputId,
-                           label="Download",
-                           class=NULL,
+                           label = "Download",
+                           class = NULL,
                           ...,
                           enabled = c("auto", TRUE, FALSE),
                           icon = shiny::icon("download")) {
@@ -1302,19 +1303,19 @@ downloadButton <- function(outputId,

 #' @rdname downloadButton
 #' @export
-downloadLink <- function(outputId, label="Download", class=NULL, ...,
+downloadLink <- function(outputId, label = "Download", class = NULL, ...,
                         enabled = c("auto", TRUE, FALSE)) {
  enabled <- match.arg(as.character(enabled), c("auto", "TRUE", "FALSE"))
-  tags$a(id=outputId,
-         class="shiny-download-link",
-         class=if (enabled != "TRUE") "disabled",
-         class=class,
-         href='',
-         target='_blank',
-         download=NA,
-         "aria-disabled"=if (enabled != "TRUE") "true" else NULL,
-         "data-ignore-update"=if (enabled == "FALSE") NA else NULL,
-         tabindex=if (enabled != "TRUE") "-1" else NULL,
+  tags$a(id = outputId,
+         class = "shiny-download-link",
+         class = if (enabled != "TRUE") "disabled",
+         class = class,
+         href = '',
+         target = '_blank',
+         download = NA,
+         "aria-disabled" = if (enabled != "TRUE") "true" else NULL,
+         "data-ignore-update" = if (enabled == "FALSE") NA else NULL,
+         tabindex = if (enabled != "TRUE") "-1" else NULL,
         label, ...)
 }

--- a/inst/www/shared/shiny.js
+++ b/inst/www/shared/shiny.js
@@ -3023,10 +3023,8 @@
    }
    // Progress shouldn't be shown on the download button
    // (progress will be shown as a page level pulse instead)
-    showProgress(el, show3) {
+    showProgress() {
      return;
-      el;
-      show3;
    }
  };
  (0, import_jquery25.default)(document).on(
--- a/inst/www/shared/shiny.js.map
+++ b/inst/www/shared/shiny.js.map
--- a/inst/www/shared/shiny.min.js
+++ b/inst/www/shared/shiny.min.js
--- a/inst/www/shared/shiny.min.js.map
+++ b/inst/www/shared/shiny.min.js.map
--- a/man/downloadButton.Rd
+++ b/man/downloadButton.Rd
@@ -38,9 +38,13 @@ is assigned to.}
 enabled when the \code{downloadHandler} provides a non-\code{NULL} filename.
 \item \code{TRUE}: the button starts enabled immediately, without waiting for the
 \code{downloadHandler}.
-\item \code{FALSE}: the button remains disabled and Shiny will not automatically
-enable it; you must manage the enabled/disabled state yourself (e.g.,
-with \code{\link[shinyjs:enable]{shinyjs::enable()}} and \code{\link[shinyjs:disable]{shinyjs::disable()}}).
+\item \code{FALSE}: the button starts disabled and Shiny will \strong{never}
+automatically enable it, even after the \code{downloadHandler} is ready.
+This opt-out applies for the lifetime of the page: \code{renderValue} will
+always skip the auto-enable step for this element, regardless of its
+current runtime state. You are responsible for managing the
+enabled/disabled state yourself (e.g., with \code{\link[shinyjs:stateFuncs]{shinyjs::enable()}} and
+\code{\link[shinyjs:stateFuncs]{shinyjs::disable()}}).
 }}

 \item{icon}{An \code{\link[=icon]{icon()}} to appear on the button. Default is \code{icon("download")}.}
--- a/man/plotPNG.Rd
+++ b/man/plotPNG.Rd
@@ -41,7 +41,7 @@ set to \code{FALSE}), then use \code{\link[ragg:agg_png]{ragg::agg_png()}}.
 \item If a quartz device is available (i.e., \code{capabilities("aqua")} is
 \code{TRUE}), then use \code{png(type = "quartz")}.
 \item If the Cairo package is installed (and the \code{shiny.usecairo} option
-is not set to \code{FALSE}), then use \code{\link[Cairo:Cairo]{Cairo::CairoPNG()}}.
+is not set to \code{FALSE}), then use \code{\link[Cairo:CairoPNG]{Cairo::CairoPNG()}}.
 \item Otherwise, use \code{\link[grDevices:png]{grDevices::png()}}. In this case, Linux and Windows
 may not antialias some point shapes, resulting in poor quality output.
 }
--- a/srcts/src/bindings/output/downloadlink.ts
+++ b/srcts/src/bindings/output/downloadlink.ts
@@ -22,10 +22,8 @@ class DownloadLinkOutputBinding extends OutputBinding {
  }
  // Progress shouldn't be shown on the download button
  // (progress will be shown as a page level pulse instead)
-  showProgress(el: HTMLElement, show: boolean): void {
+  showProgress(): void {
    return;
-    el; // eslint-disable-line @typescript-eslint/no-unused-expressions
-    show; // eslint-disable-line @typescript-eslint/no-unused-expressions
  }
 }

--- a/srcts/types/src/bindings/output/downloadlink.d.ts
+++ b/srcts/types/src/bindings/output/downloadlink.d.ts
@@ -2,6 +2,6 @@ import { OutputBinding } from "./outputBinding";
 declare class DownloadLinkOutputBinding extends OutputBinding {
    find(scope: HTMLElement): JQuery<HTMLElement>;
    renderValue(el: HTMLElement, data: string): void;
-    showProgress(el: HTMLElement, show: boolean): void;
+    showProgress(): void;
 }
 export { DownloadLinkOutputBinding };
--- a/tests/testthat/apps/download-button/app.R
+++ b/tests/testthat/apps/download-button/app.R
@@ -0,0 +1,115 @@
+library(shiny)
+
+# This app covers the three `enabled` values for both downloadButton and downloadLink,
+# plus a simulated shinyjs-disabled case, with toggle buttons for each scenario.
+
+# Create a downloadHandler that just serves a simple text file for testing.
+handler <- function() {
+  downloadHandler(
+    filename = "test.txt",
+    content = function(file) writeLines("test", file)
+  )
+}
+
+# Mirrors what shinyjs::enable() / shinyjs::disable() does: adds/removes the
+# shinyjs-disabled class and the standard disabled attributes. This lets us test
+# that the download buttons/links respond to external JS changes to their enabled state,
+# without making us require shinyjs .
+set_enabled_js <- HTML(
+  "
+  Shiny.addCustomMessageHandler('setEnabled', function(data) {
+    var el = document.getElementById(data.id);
+    if (data.enabled) {
+      el.classList.remove('disabled');
+      el.classList.remove('shinyjs-disabled');
+      el.removeAttribute('aria-disabled');
+      el.removeAttribute('tabindex');
+    } else {
+      el.classList.add('disabled');
+      el.classList.add('shinyjs-disabled');
+      el.setAttribute('aria-disabled', 'true');
+      el.setAttribute('tabindex', '-1');
+    }
+  });
+"
+)
+
+ui <- fluidPage(
+  tags$script(set_enabled_js),
+  # Block of download Button tests
+
+  h3("downloadButton"),
+
+  downloadButton("btn_auto", "Auto (default)"),
+  actionButton("toggle_btn_auto", "Toggle"),
+
+  downloadButton("btn_off", "Disabled", enabled = FALSE),
+  actionButton("toggle_btn_off", "Toggle"),
+
+  downloadButton("btn_on", "Pre-enabled", enabled = TRUE),
+  actionButton("toggle_btn_on", "Toggle"),
+
+  # This mimics what happens when a download button is wrapped in a
+  # shinyjs::disabled() call within the UI (and therefore at render time).
+  htmltools::tagAppendAttributes(
+    downloadButton("btn_shinyjs", "shinyjs-disabled"),
+    class = "shinyjs-disabled"
+  ),
+  actionButton("toggle_btn_shinyjs", "Toggle"),
+
+  # Block of download Link tests
+  h3("downloadLink"),
+
+  downloadLink("lnk_auto", "Auto (default)"),
+  actionButton("toggle_lnk_auto", "Toggle"),
+
+  downloadLink("lnk_off", "Disabled", enabled = FALSE),
+  actionButton("toggle_lnk_off", "Toggle"),
+
+  downloadLink("lnk_on", "Pre-enabled", enabled = TRUE),
+  actionButton("toggle_lnk_on", "Toggle"),
+
+  htmltools::tagAppendAttributes(
+    downloadLink("lnk_shinyjs", "shinyjs-disabled"),
+    class = "shinyjs-disabled"
+  ),
+  actionButton("toggle_lnk_shinyjs", "Toggle")
+)
+
+server <- function(input, output, session) {
+  output$btn_auto <- handler()
+  output$btn_off <- handler()
+  output$btn_on <- handler()
+  output$btn_shinyjs <- handler()
+  output$lnk_auto <- handler()
+  output$lnk_off <- handler()
+  output$lnk_on <- handler()
+  output$lnk_shinyjs <- handler()
+
+  # Each reactiveVal tracks the current intended state, starting from
+  # the post-render-value state (auto/on start enabled; off/shinyjs start disabled).
+  make_toggle <- function(id, initial_enabled) {
+    enabled <- reactiveVal(initial_enabled)
+    observeEvent(input[[paste0("toggle_", id)]], {
+      new_state <- !enabled()
+      enabled(new_state)
+      # This mimics what shinyjs::enable()/disable() would do, which is to send
+      # a message to the client to update the button's enabled state via JS.
+      session$sendCustomMessage(
+        "setEnabled",
+        list(id = id, enabled = new_state)
+      )
+    })
+  }
+
+  make_toggle("btn_auto", TRUE)
+  make_toggle("btn_off", FALSE)
+  make_toggle("btn_on", TRUE)
+  make_toggle("btn_shinyjs", FALSE)
+  make_toggle("lnk_auto", TRUE)
+  make_toggle("lnk_off", FALSE)
+  make_toggle("lnk_on", TRUE)
+  make_toggle("lnk_shinyjs", FALSE)
+}
+
+shinyApp(ui, server)
--- a/tests/testthat/test-downloadButton-shinytest2.R
+++ b/tests/testthat/test-downloadButton-shinytest2.R
@@ -0,0 +1,231 @@
+skip_if_not_installed("shinytest2")
+skip_if_not_installed("callr")
+library(shinytest2)
+
+# Start the test app in a subprocess, loading the local dev shiny.
+# AppDriver$new(url) is used instead of AppDriver$new(app_dir) because the
+# latter's internal subprocess runner has a timing issue with devtools-loaded
+# packages that prevents the shinytest2 tracer from detecting jQuery.
+app_process <- callr::r_bg(
+  function(app_file, port) {
+    devtools::load_all(quiet = TRUE)
+    shiny::runApp(
+      shiny::shinyAppFile(app_file),
+      port = port,
+      host = "127.0.0.1",
+      launch.browser = FALSE,
+      quiet = TRUE
+    )
+  },
+  args = list(
+    app_file = testthat::test_path("apps/download-button/app.R"),
+    port = 7314L
+  )
+)
+withr::defer(app_process$kill())
+
+# Wait for the app to be ready
+for (i in seq_len(20)) {
+  Sys.sleep(0.5)
+  ready <- tryCatch(
+    {
+      httr::GET("http://127.0.0.1:7314", httr::timeout(1))
+      TRUE
+    },
+    error = function(e) FALSE
+  )
+  if (ready) break
+}
+if (!ready) skip("Download button test app failed to start")
+
+app_url <- "http://127.0.0.1:7314"
+
+is_disabled <- function(app, id) {
+  app$get_js(sprintf(
+    "var el = document.querySelector('#%s');
+     el.classList.contains('disabled') &&
+     el.getAttribute('aria-disabled') === 'true' &&
+     el.getAttribute('tabindex') === '-1';", id
+  ))
+}
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+click_toggle <- function(app, id) {
+  app$click(input = paste0("toggle_", id), wait_ = FALSE)
+  app$wait_for_idle()
+}
+
+# ---------------------------------------------------------------------------
+# downloadButton
+# ---------------------------------------------------------------------------
+
+test_that("downloadButton (enabled='auto') auto-enables after server init", {
+  app <- AppDriver$new(app_url, name = "dl-button-auto")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_false(is_disabled(app, "btn_auto"))
+  expect_null(app$get_js("document.querySelector('#btn_auto').getAttribute('aria-disabled')"))
+})
+
+test_that("downloadButton (enabled=FALSE) stays disabled after server init", {
+  app <- AppDriver$new(app_url, name = "dl-button-off")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_true(is_disabled(app, "btn_off"))
+})
+
+test_that("downloadButton (enabled=TRUE) starts and stays enabled", {
+  app <- AppDriver$new(app_url, name = "dl-button-on")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_false(is_disabled(app, "btn_on"))
+  expect_null(app$get_js("document.querySelector('#btn_on').getAttribute('aria-disabled')"))
+})
+
+test_that("downloadButton with shinyjs-disabled class stays disabled after server init", {
+  app <- AppDriver$new(app_url, name = "dl-button-shinyjs")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_true(is_disabled(app, "btn_shinyjs"))
+})
+
+test_that("downloadButton (enabled='auto') can be toggled off and back on", {
+  app <- AppDriver$new(app_url, name = "dl-button-auto-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "btn_auto")
+  expect_true(is_disabled(app, "btn_auto"))
+
+  click_toggle(app, "btn_auto")
+  expect_false(is_disabled(app, "btn_auto"))
+})
+
+test_that("downloadButton (enabled=FALSE) can be toggled on and back off", {
+  app <- AppDriver$new(app_url, name = "dl-button-off-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "btn_off")
+  expect_false(is_disabled(app, "btn_off"))
+
+  click_toggle(app, "btn_off")
+  expect_true(is_disabled(app, "btn_off"))
+})
+
+test_that("downloadButton (enabled=TRUE) can be toggled off and back on", {
+  app <- AppDriver$new(app_url, name = "dl-button-on-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "btn_on")
+  expect_true(is_disabled(app, "btn_on"))
+
+  click_toggle(app, "btn_on")
+  expect_false(is_disabled(app, "btn_on"))
+})
+
+test_that("downloadButton (shinyjs-disabled) can be toggled on and back off", {
+  app <- AppDriver$new(app_url, name = "dl-button-shinyjs-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "btn_shinyjs")
+  expect_false(is_disabled(app, "btn_shinyjs"))
+
+  click_toggle(app, "btn_shinyjs")
+  expect_true(is_disabled(app, "btn_shinyjs"))
+})
+
+# ---------------------------------------------------------------------------
+# downloadLink
+# ---------------------------------------------------------------------------
+
+test_that("downloadLink (enabled='auto') auto-enables after server init", {
+  app <- AppDriver$new(app_url, name = "dl-link-auto")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_false(is_disabled(app, "lnk_auto"))
+  expect_null(app$get_js("document.querySelector('#lnk_auto').getAttribute('aria-disabled')"))
+})
+
+test_that("downloadLink (enabled=FALSE) stays disabled after server init", {
+  app <- AppDriver$new(app_url, name = "dl-link-off")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_true(is_disabled(app, "lnk_off"))
+})
+
+test_that("downloadLink (enabled=TRUE) starts and stays enabled", {
+  app <- AppDriver$new(app_url, name = "dl-link-on")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_false(is_disabled(app, "lnk_on"))
+  expect_null(app$get_js("document.querySelector('#lnk_on').getAttribute('aria-disabled')"))
+})
+
+test_that("downloadLink with shinyjs-disabled class stays disabled after server init", {
+  app <- AppDriver$new(app_url, name = "dl-link-shinyjs")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  expect_true(is_disabled(app, "lnk_shinyjs"))
+})
+
+test_that("downloadLink (enabled='auto') can be toggled off and back on", {
+  app <- AppDriver$new(app_url, name = "dl-link-auto-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "lnk_auto")
+  expect_true(is_disabled(app, "lnk_auto"))
+
+  click_toggle(app, "lnk_auto")
+  expect_false(is_disabled(app, "lnk_auto"))
+})
+
+test_that("downloadLink (enabled=FALSE) can be toggled on and back off", {
+  app <- AppDriver$new(app_url, name = "dl-link-off-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "lnk_off")
+  expect_false(is_disabled(app, "lnk_off"))
+
+  click_toggle(app, "lnk_off")
+  expect_true(is_disabled(app, "lnk_off"))
+})
+
+test_that("downloadLink (enabled=TRUE) can be toggled off and back on", {
+  app <- AppDriver$new(app_url, name = "dl-link-on-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "lnk_on")
+  expect_true(is_disabled(app, "lnk_on"))
+
+  click_toggle(app, "lnk_on")
+  expect_false(is_disabled(app, "lnk_on"))
+})
+
+test_that("downloadLink (shinyjs-disabled) can be toggled on and back off", {
+  app <- AppDriver$new(app_url, name = "dl-link-shinyjs-toggle")
+  on.exit(app$stop())
+  app$wait_for_idle()
+
+  click_toggle(app, "lnk_shinyjs")
+  expect_false(is_disabled(app, "lnk_shinyjs"))
+
+  click_toggle(app, "lnk_shinyjs")
+  expect_true(is_disabled(app, "lnk_shinyjs"))
+})
--- a/tests/testthat/test-downloadButton.R
+++ b/tests/testthat/test-downloadButton.R
@@ -8,6 +8,16 @@ test_that("downloadButton starts disabled with correct attributes", {
  expect_match(html, 'href=""')
 })

+test_that("downloadLink starts disabled with correct attributes", {
+  lnk <- downloadLink("dl", "Download")
+  html <- as.character(lnk)
+
+  expect_match(html, "class=.*disabled")
+  expect_match(html, 'aria-disabled="true"')
+  expect_match(html, 'tabindex="-1"')
+  expect_match(html, 'href=""')
+})
+
 test_that("downloadButton omits data-ignore-update by default (enabled = 'auto')", {
  btn <- downloadButton("dl", "Download")
  html <- as.character(btn)