daattali / shinyalert

🗯️ Easily create pretty popup messages (modals) in Shiny
https://daattali.com/shiny/shinyalert-demo/
Other
241 stars 26 forks source link

[Bug] Cannot access input variable for shinyalert modal created in `callbackR` #46

Closed JamesBrownSEAMS closed 4 months ago

JamesBrownSEAMS commented 3 years ago

I have a simple user interface that asks two questions and takes two user defined values as responces. I have implmented this using the latest version of shinyalert using the new features that allow input controls to be defined in the modals. An example of the process is shown here:

library(shiny)
library(shinyalert)

ui <- fluidPage(
  useShinyalert(),

  tags$h2("Demo shinyalert"),
  tags$br(),
  fluidRow(
    column(
      width = 6,
      actionButton("shinyalert_popup",
                   "Alert Popup")
    )
  ),
  fluidRow(
    verbatimTextOutput(outputId = "return_value_1"),
    verbatimTextOutput(outputId = "return_value_2")
  )
)

server <- function(input, output, session) {

  observeEvent(input$shinyalert_popup, {
    shinyalert(
      title = "Input Number 1",
      text = tagList(
        "Enter a number",
        textInput(inputId = "shinyalert_value_1",
                  label = "")
      ),
      html = TRUE,
      inputId = "shinyalert_modal",
      showConfirmButton = TRUE,
      confirmButtonText = "OK",
      callbackR = function() {
        shinyalert(
          title = "Input Number 2",
          text = tagList(
            "Enter another number",
            textInput(inputId = "shinyalert_value_2",
                      label = "")
          ),
          html = TRUE,
          inputId = "shinyalert_callback",
          showConfirmButton = TRUE,
          confirmButtonText = "OK"
        )
      }
    )
  })

  output$return_value_1 <- renderPrint(input$shinyalert_value_1)
  output$return_value_2 <- renderPrint(input$shinyalert_value_2)
}

shinyApp(ui = ui, server = server)

The responce shows that the first input, from the id shinyalert_value_1 is captured and updates in the ui. The second input, from shinyalert_value_2, which is nested in the callbackR parameter shows a NULL when values are typed and after the modal has been closed. This indicates that the input$shinyalert_value_2 is not found in this configuration.

daattali commented 3 years ago

Thanks for reporting @JamesBrownSEAMS

I can confirm this is indeed a bug. It seems to do with the timing, perhaps the new input is being added and unbound too early or something like that, because if I add a 500ms delay before showing the second alert then it works, but a 100ms delay does not. I'm caught up in a lot of projects and not sure when I'd be able to debug this, I would warmly welcome any help.

JamesBrownSEAMS commented 3 years ago

I created a modified example following comments about the delay and used shinyjs to add one for testing:

library(shiny)
library(shinyalert)
library(shinyjs)

ui <- fluidPage(
  useShinyalert(),
  useShinyjs(),

  tags$h2("Demo shinyalert"),
  tags$br(),
  fluidRow(
    column(
      width = 6,
      actionButton("shinyalert_popup",
                   "Alert Popup")
    )
  ),
  fluidRow(
    verbatimTextOutput(outputId = "return_value_1"),
    verbatimTextOutput(outputId = "return_value_2"),
    verbatimTextOutput(outputId = "return_value_3")
  )
)

server <- function(input, output, session) {

  observeEvent(input$shinyalert_popup, {
    shinyalert(
      title = "Input Number 1",
      text = tagList(
        "Enter a number",
        textInput(inputId = "shinyalert_value_1",
                  label = ""),
        "Enter another number",
        textInput(inputId = "shinyalert_value_2",
                  label = "")
      ),
      html = TRUE,
      inputId = "shinyalert_modal",
      showConfirmButton = TRUE,
      confirmButtonText = "OK",
      callbackR = function() {
        delay(371,
              shinyalert(
                title = "Input Number 3",
                text = tagList(
                  "Enter a final number",
                  textInput(inputId = "shinyalert_value_3",
                            label = "")
                ),
                html = TRUE,
                inputId = "shinyalert_callback",
                showConfirmButton = TRUE,
                confirmButtonText = "OK"
              )
        )
      }
    )
  })

  output$return_value_1 <- renderPrint(input$shinyalert_value_1)
  output$return_value_2 <- renderPrint(input$shinyalert_value_2)
  output$return_value_3 <- renderPrint(input$shinyalert_value_3)
}

shinyApp(ui = ui, server = server)

I narrowed the threshold between the input being available or not to 370ms. At a delay of 370ms the input shinyalert_value_3 cannot be accessed, at 371ms it can. Tested this on a couple of machines and results were consistnet in each case. I have done a bit of research but can't find anything that indicates 370ms is a significant value for anything.

JamesBrownSEAMS commented 3 years ago

Further testing on the delay. Cutoffs for the major browsers:

daattali commented 3 years ago

Thanks for the testing. Though I'd be inclined to ignore the exact values, and instead look into solving the underlying problem rther than add a delay :)

JamesBrownSEAMS commented 3 years ago

Agreed, I wanted to see if there was a consistent delay requirement that might point to some process in the browser (perhaps in JavaScript) that was causing the issue. Alas, I cannot find anything online that indicates there is anything special about these timings.

daattali commented 3 years ago

That's interesting indeed that it's so consistent, I didn't expect that!

JamesBrownSEAMS commented 3 years ago

Indeed, I had expected it to be semi-random within a deteminable range but I reran the tests a number of times under different conditions for the R environment and the numbers are consistent for each browser. This is what is making me think there is something at play that runs in the browser when the modal pops up that impacts the accessibility of the input value. Unfortunatly I have no idea what that might be...

stla commented 2 years ago

For a textInput, you can use the one of SweetAlert:

      callbackR = function() {
        shinyalert(
          type = "input",
          title = "Input Number 2",
          text = "Enter another number",
          inputId = "shinyalert_value_2",
          showConfirmButton = TRUE,
          confirmButtonText = "OK"
        )
      }
stla commented 2 years ago

Or you can use the JS callback:

      callbackR = function() {
        shinyalert(
          title = "Input Number 2",
          text = tagList(
            "Enter another number",
            textInput(inputId = "shinyalert_value_2",
                      label = "")
          ),
          callbackJS = 
            "function(){setTimeout(function(){var x = $('#shinyalert_value_2').val(); Shiny.setInputValue('shinyalert_value_2', x)});}",
          html = TRUE,
          inputId = "shinyalert_callback",
          showConfirmButton = TRUE,
          confirmButtonText = "OK"
        )
      }
stla commented 2 years ago

Other solution, with Shiny unbind/bind:

    shinyalert(
      title = "Input Number 1",
      text = tagList(
        "Enter a number",
        textInput(inputId = "shinyalert_value_1",
                  label = "")
      ),
      html = TRUE,
      inputId = "shinyalert_modal",
      showConfirmButton = TRUE,
      confirmButtonText = "OK",
      callbackJS = "function(){Shiny.unbindAll();}",
      callbackR = function() {
        shinyalert(
          title = "Input Number 2",
          text = tagList(
            "Enter another number",
            textInput(inputId = "shinyalert_value_2",
                      label = "")
          ),
          callbackJS = "function(){Shiny.bindAll();}",
          html = TRUE,
          inputId = "shinyalert_callback",
          showConfirmButton = TRUE,
          confirmButtonText = "OK"
        )
      }
    )
daattali commented 2 years ago

@stla thanks for the input, that's useful! Perhaps unbind/bind is the key to solving this issue

stla commented 1 year ago

Related: https://stackoverflow.com/q/76251926/1100107.

daattali commented 1 year ago

Thanks for finding this strange workaround. This issue remains one of the most, if not the most, severe bug in all of my packages. If anyone is able to find the issue and a clean solution I would be very grateful!

ch0c0l8ra1n commented 5 months ago

@daattali If 100ms doesn't work and 500ms works, its probably related to #48

The problem is how sweetalert closing is defined.

initialize: function() {
  var service = this;
  var originalClose = this.__swal.close;
  this.__swal.close = function() {
    service.isClosing = true;
    originalClose();
    service.currentSwal = null;
    setTimeout(function() {
      service.onClosed();
    }, 400);
  };
}

There's a setTimeout that calls a function 400ms after you call alert.close().

daattali commented 5 months ago

I never noticed that swalservice has this delay. That seems a bit irresponsible, to place that in a publicly used library, it seems like it's bound to create issues. Any idea why it's there, and if it's safe to change it to 0?

ch0c0l8ra1n commented 5 months ago

@daattali The delay is there to help with animations as seen in sweetalert.dev.js which was not provided with shinyalert.

image

I can think of a pretty straightforward solution to handle pending swals that makes minor edits to the underlying library. We can make it so that swalservice waits till old sweet alert is completely closed before displaying the new sweet alert.

daattali commented 5 months ago

I said earlier that I wouldn't want to modify the code of an external library, but if it's a small easy to understand change that has only a positive effect, and will be able to close multiple open issues, then yes please :)

ch0c0l8ra1n commented 4 months ago

@daattali the issue, while related to the setTimeout, turned out to be slightly different than what I had thought. It was a problem of shiny not binding the inputs for swals that were queued by swalservice.js

It's simply fixed by adding a Shiny.bindAll after opening queued swals.

86 should fix this and #48

daattali commented 4 months ago

Fixed in https://github.com/daattali/shinyalert/commit/510bb42fc605a9c9bfef9ff05b361c30310270a3 - thanks to @stla and @ch0c0l8ra1n