rstudio / shiny

Easy interactive web applications with R
https://shiny.posit.co/
Other
5.37k stars 1.87k forks source link

Persistent `.recalculating` class if removeUI and output hiding operations are performed #4114

Closed gsmolinski closed 2 months ago

gsmolinski commented 2 months ago

System details

R version 4.4.1 (2024-06-14 ucrt)
Platform: x86_64-w64-mingw32/x64
Running under: Windows 10 x64 (build 19044)

Matrix products: default

locale:
[1] LC_COLLATE=Polish_Poland.utf8 
[2] LC_CTYPE=Polish_Poland.utf8   
[3] LC_MONETARY=Polish_Poland.utf8
[4] LC_NUMERIC=C                  
[5] LC_TIME=Polish_Poland.utf8    

time zone: Europe/Warsaw
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices utils     datasets 
[6] methods   base     

other attached packages:
[1] reactable_0.4.4 shiny_1.9.1    

loaded via a namespace (and not attached):
 [1] cli_3.6.3         rlang_1.1.4      
 [3] promises_1.3.0    jsonlite_1.8.8   
 [5] xtable_1.8-4      listenv_0.9.1    
 [7] htmltools_0.5.8.1 httpuv_1.6.15    
 [9] reactR_0.6.0      sass_0.4.9.9000  
[11] crosstalk_1.2.1   jquerylib_0.1.4  
[13] fastmap_1.2.0     yaml_2.3.8       
[15] lifecycle_1.0.4   memoise_2.0.1    
[17] compiler_4.4.1    codetools_0.2-20 
[19] htmlwidgets_1.6.4 Rcpp_1.0.12      
[21] rstudioapi_0.16.0 future_1.33.2    
[23] later_1.3.2       digest_0.6.36    
[25] R6_2.5.1          parallelly_1.37.1
[27] parallel_4.4.1    magrittr_2.0.3   
[29] bslib_0.7.0       tools_4.4.1      
[31] withr_3.0.0       mime_0.12        
[33] globals_0.16.3    cachem_1.1.0     

Example application or steps to reproduce the problem

To reproduce the problem:

  1. Run app.
  2. Click "Hide"
  3. Click "Remove UI"
  4. Click "Show"
  5. Change input text to something different

You will see persistent busy indicator.

library(shiny)

ui <- fluidPage(
  tags$script(HTML("
$( document ).ready(function() {
 Shiny.addCustomMessageHandler('show', function(e) {
   let result = document.getElementById('text_1');
   result.style.removeProperty('display');
 });
});
                   ")),
  tags$script(HTML("
 $( document ).ready(function() {
 Shiny.addCustomMessageHandler('hide', function(e) {
   let result = document.getElementById('text_1');
      if (result != null) {
     result.style.display = 'none';
   }
 });
});
                   ")),
  useBusyIndicators(),
  div(id = "my_div"),
  textInput("text_input", "Text", value = "test"),
  actionButton("hide_btn", "Hide"),
  actionButton("rm_btn", "Remove UI"),
  actionButton("show_btn", "Show")
)

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

  observe({
    insertUI("#my_div", ui = textOutput("text_1"))
    insertUI("#my_div", ui = textOutput("text_2"))
  })

  observe({
    removeUI("#text_2")
    output[["text_2"]] <- NULL
  }) |>
    bindEvent(input$rm_btn)

  observe({
    session$sendCustomMessage("hide", "placeholder")
  }) |>
    bindEvent(input$hide_btn)

  observe({
    session$sendCustomMessage("show", "placeholder")
  }) |>
    bindEvent(input$show_btn)

  output$text_1 <- renderText({
    input$text_input
  })

  output$text_2 <- renderText({
    "this line will be removed"
  })

}

shinyApp(ui, server)

Describe the problem in detail

I'm working on the app where user can create outputs dynamically, remove them, hide them and show them; and above I have showed as minimal reproducible example as I was able to produce based on my app.

Problem is that busy indicator (class .recalculating) is persistent if UI removing (of output X) and UI hiding (of output Y) is used (both of these operations must be used and in the order I mentioned above). Specifically, problem is when output X is removing and at the same time output Y has style display = 'none'.

I would like to say that in the previous version of shiny this problem does not occur, but this is somehow more complicated. In my real app this problem does not occur in previous version, but using this MRE I see the similar problem (of course, busy indicator is not present, but output is not updated in previous version where busy indicators were not introduced). I can't say why in my real app this problem was not present in previous version.

I believe this is a bug in {shiny}, but if someone could explain me, why this problem exists and what can I do with that, that would be great :).

cpsievert commented 2 months ago

I haven't fully investigated this yet, but it also seems important to note that the behavior of this app doesn't seem correct with shiny v1.8.1.1 either (before busy indicators and changes to .recalculating). With that version of shiny (v1.8.1.1), and by following the steps outlined here, one would expect the text_1 output to update when the input changes, but it doesn't:

https://github.com/user-attachments/assets/3ae7adfa-daf2-43b6-b78a-0c86f13ed67a

So, I'm not yet ready to say the new behavior is "wrong" to label the output as recalculating, but is more likely highlighting a problem that already existed

cpsievert commented 2 months ago

Ok, I've investigated the issue further, and it's due to the fact that the show/hide methods used here are changing the visibility of text_1 without letting Shiny know about that change. You can fix that issue by updating your show message handler to:

Shiny.addCustomMessageHandler('show', function(e) {
   let result = document.getElementById('text_1');
   result.style.removeProperty('display');
   Shiny.bindAll(result)
});
cpsievert commented 2 months ago

Count this as yet another reason why Shiny should be leveraging IntersectionObserver() and ResizeObserver() to automatically send visibility and size changes to the server. There's a draft for this in https://github.com/rstudio/shiny/pull/3682

gsmolinski commented 2 months ago

Thank you very much! I wasn't aware that it is need to call bindAll().