Allow creating disabled `downloadButton()`/`downloadLink()`

[riccardoporreca]

Including a downloadButton() (similar for downloadLink()) disabled explicitly in the app UI might be convenient for cases where the download is only available after some user interaction.

Before #4041, it was possible to add a "disabled" class when creating a downloadButton() in the UI (and then explicitly re-enable in the app server logic using e.g. shinyjs), but now the extra "disabled" class is always removed:

https://github.com/rstudio/shiny/blob/d84aa94762b4ffaf7533a007b6cb92c40f4f29af/srcts/src/bindings/output/downloadlink.ts#L10-L13

Maybe a nice way to resolve this would be to have built-in support for creating a disabled downloadButton() with disabled = TRUE , similar to actionButton(), with a similar mechanism to updateActionButton() for enabling/disabling it as part of the server logic?

[daattali]

@cpsievert is there any realistic chance this behaviour will be rectified? The {shinyjs} package has a disabled() function that initializes inputs as disabled, and this commit broke the functionality for download buttons. See https://github.com/daattali/shinyjs/issues/277

@riccardoporreca I'm very curious if you ran into this issue in shinyjs, or did you have your own custom code that was disabling download buttons?

[daattali]

In {shinyjs} I add custom classes on inputs so that later on in the JS layer, I could look at the classes to decide what to do with the input.

I would be happy to submit a PR that checks to make sure the download button isn't explicitly disabled before enabling it. The downsize of that PR is that it would add code that is only there to support a specific third party package, which I know is not ideal.

[riccardoporreca]

@daattali

I'm very curious if you ran into this issue in shinyjs, or did you have your own custom code that was disabling download buttons?

I ran into this in Shiny for Python, actually, where we are relying session.send_custom_message to invoke a JavaScript code that would add/remove classes reactively based on the app state. (For completeness, the issue introduced by #4041 was ported to py-shiny with https://github.com/posit-dev/py-shiny/pull/1353.)

We ended up using a custom class and corresponding CSS for disabling download buttons as a workaround.

I would be happy to submit a PR that checks to make sure the download button isn't explicitly disabled before enabling it. The downsize of that PR is that it would add code that is only there to support a specific third party package, which I know is not ideal.

I would agree on this solution not being ideal, which makes me go back to what I sort of hinted above:

Maybe a nice way to resolve this would be to have built-in support for creating a disabled downloadButton() with disabled = TRUE , similar to actionButton(), with a similar mechanism to updateActionButton() for enabling/disabling it as part of the server logic?

With this, one would not even need to use shinyjs or any other custom code for disabling/enabling a download button.

[daattali]

I had no idea actionButton() received a disabled argument! It didn't make it into the NEWS so I never noticed.

Since that button has a disabled, that certainly gives precedence to having the argument for downloadButton() as well. I'm very curious why it was added to actionButton() and only to it, I don't see any issue associated with the commit or any reason given.

[daattali]

After thinking about it, I suppose they might have added this functionality to the button in order to support the input task button. That makes sense! But IMO it's not a clean solution to introduce a useful parameter to only one input that shiny happens to need -- if support is being added, then add it across the board to all inputs so that other usescases can also take advantage of disabling/enabling. Or add it behind the scenes in a way that isn't exposed to the user. It feels weird to me as a user that a feature that I would like to use in many functions is only allowed in one case.

[riccardoporreca]

@daattali, I think there is a big difference with download button/link: Those are not inputs but outputs, and as such undergo a pretty different handling compared to an input actionButton, which I guess would also make support for updateDownloadButton not necessarily straightforward to implement.

I guess at this stage we should mostly try to get an opinion from Shiny's maintainers (@cpsievert) about how we can move forward with the issue. I can give it some more thoughts at my end but would like to first get some input from their side.

[gadenbuie]

Including a downloadButton() (similar for downloadLink()) disabled explicitly in the app UI might be convenient for cases where the download is only available after some user interaction.

@riccardoporreca I'm sorry for the delay in responding to your original question!

Here's a solution to your issue: you can use uiOutput() and renderUI() (or @render.ui in Shiny for Python) to show a disabled action button prior to the user interaction that is replaced with the download button when ready. Here's a small example (that you can try on shinylive.io):

library(shiny)
library(bslib)

ui <- page_fillable(
  textInput("filename", "File Name"),
  uiOutput("ui_download")
)

server <- function(input, output) {
  output$ui_download <- renderUI({
    if (!isTruthy(input$filename)) {
      actionButton(
        "download_disabled",
        "Provide a file name to download",
        icon = icon("download"),
        disabled = TRUE
      )
    } else {
      downloadButton(
        "download",
        HTML(sprintf("Download <code>%s.csv</code>", input$filename))
      )
    }
  })

  output$download <- downloadHandler(
    filename = function() {
      paste0(input$filename, ".csv")
    },
    content = function(file) {
      write.csv(mtcars, file)
    }
  )
}

shinyApp(ui = ui, server = server)

I think @daattali is right about why and how disabled was added to actionButton() (as part of supporting input_task_button()). We disable the download button/link during load because many browsers will download the current webpage if we have't registered our handlers yet. We don't have any plans at the moment to add support for disabling inputs other than action buttons.

@cpsievert is there any realistic chance this behaviour will be rectified? The {shinyjs} package has a disabled() function that initializes inputs as disabled, and this commit broke the functionality for download buttons. See daattali/shinyjs#277

I would say there's a small chance that we'll change this. downloadButton() isn't built on actionButton(), but I think there's a reasonable argument that we could have a similar disabled argument for download buttons.

That said, we won't be able to know the source of the disabled class so we'd have to move the intent to another class or data attribute, and it's hard for me to see a path forward that's compatible with shinyjs::disable() as-is.

We could move the "user disabled download button" intent into a data attribute (or a class), e.g.

<a class="shiny-download-link disabled" data-user-disabled></a>

when disabled=TRUE. Then we'd skip enabling the button on load to wait for the user to manually enable the button.

Or we could do the reverse and include an attribute by default. In either case, we can'y rely entirely on the presence or absence of the disabled class and the best-case scenario for shinyjs::disabled() would be that it needs to learn how to add our data attribute to download links.

This doesn't feel like a great solution so for now, I'd recommend using uiOutput() or other means -- e.g. validate(needs(condition, "Provide a file name")) -- to put informative placeholder content where the download button will be when the conditions are right.

[daattali]

@gadenbuie I realize none of the potential solutions you suggested are ideal, I 100% understand your pain in not wanting to check for an additional class/attribute. But I'm conflicted about this because the new behaviour of automatically enabling all download buttons also seems non-ideal. So I'd argue that it should have an escape hatch. I wouldn't care what that escape hatch is, but having any mechanism to escape the automatic enabling seems like the correct thing to do IMO.

[riccardoporreca]

@daattali, @gadenbuie, thanks for your feedback and input!

I have tried put some concrete thoughts about this, and drafted a possible solution that inplements the disable argument for downoladButton/Link. See https://github.com/rstudio/shiny/compare/main...riccardoporreca:shiny:feature/4119-allow-disabled-download-button-link and in particular the first commit also linked above. This is currently untested and mostly a way to sketch a possible solution.

• This would allow leaving the button/link disabled, which might be valuable already even without the update* functions to provide built-in functionality in shiny to enable/disable dynamically (which might be trickier to introduce).
• As for shinyjs, this means users should not use shinyjs::disabled() in the UI but pass disabled = TRUE, and then still use shinyjs::enable()/disable() in the server. (As a related note, the enable/disable behavior for download button/link should also take care of the additional attributes, for the same principles considered in #4041.)

Happy to open a PR if it is going in the right direction, otherwise hope this is still somewhat helpful.

[gadenbuie]

I appreciate everyone's perspective and thank you for talking through suggested solutions!

I'll start with why this behavior exists in the first place.

But I'm conflicted about this because the new behaviour of automatically enabling all download buttons also seems non-ideal.

We're disabling download buttons until our server-side logic is set-up and ready to go, which avoids very disruptive problems that happened quite frequently to many, many users, where a download button would download an index.html file with the app's HTML source inadvertently.

That behavior arises out of browser behavior and we can't control it. It causes significant user pain and is usually not easy to spot as a developer.

Not being able to disable/enable the download button manually certainly causes developer friction, but at the expense of saving your app users from very unexpected and undesirable behavior.

So I'd argue that it should have an escape hatch.

The escape hatch, as I see it, is to use dynamic UI to have some placeholder text or UI in the app when the download is not yet ready or available that is then replaced with the download button when the conditions are right.

I agree this isn't as simple as manually enabling/disabling the download button, but I would also argue that the approach I've outlined is better for app users.

One of the reasons that we don't allow disabling all Shiny inputs is that disabling inputs is not a user-friendly method of communication. It's generally not accessible to users with screen readers and other assistance software, it can be very hard to see even for fully-sighted users, and it usually does not provide any communication about why the interface is disabled.

In general, we have a few solutions that are much better choices:

1. Use dynamic UI with renderUI() to get the exact UI you need in either case.
2. Use validate(need(...)) to easily replace the download button (or other UI inputs) with a helpful message.
3. Use shinyvalidate to provide validation messages and styles with the affected UI inputs.

I haven't tried to use shinyvalidate with download buttons, but I'd be happy to make sure that it works as expected or needed.

@riccardoporreca Thank you for the taking the time to sketch out a possible approach! It's definitely mostly in line with what I had in mind. The problem is that there are enough potential complications with the approach (especially in light of the above) that I see it as an option of last resort.

[cpsievert]

@riccardoporreca thanks for the draft. Unfortunately, I don't think we'd add a disable argument unless we commit to also addingupdateDownloadButton()/updateDownloadLink() which, for reasons that @gadenbuie has laid out, I'm also not keen to do at the moment.

That said, to make the situation better for {shinyjs}, maybe we could just skip the "enable-on-render" part if the <a> tag has the shinyjs-disabled class (i.e., the class that shinyjs::disabled() adds)? And {shinyjs} could be responsible for toggling the disabled class / ARIA attributes (if it doesn't already, it feels like it should)?

[riccardoporreca]

Thank you @cpsievert @gadenbuie, I appreciate a lot the detailed discussion and shared perspectives on the intended way (not) to go, which I fully understand and agree on.

Just one final input from my side on this topic. If you end up with

just skip the "enable-on-render" part if the <a> tag has the shinyjs-disabled class

would you foresee supporting also a class that is not just shinyjs-disabled to skip "enable-on-render"? This would be useful outside the R/shinyjs world. I fully understand though if the answer to this is "If not using shinyjs, you should rather go for one of the solutions that are much better choices mentioned above in https://github.com/rstudio/shiny/issues/4119#issuecomment-3201511320".

[cpsievert]

would you foresee supporting also a class that is not just shinyjs-disabled to skip "enable-on-render"? This would be useful outside the R/shinyjs world.

Probably not. I'm only offering this as a workaround since breaking existing {shiny}+{shinyjs} behavior obviously isn't what we want. If we do decide it's worth providing control of enable/disable more generally, we'd do so more officially in the shiny API, not with CSS classes.

[daattali]

I'll be happy with any solution you implement!

[al-obrien]

What was the approach forward on this then? I would hazard to say there are some legitimate patterns that a developer would want to disable buttons until certain triggers occur.