rstudio / bslib

Tools for theming Shiny and R Markdown via Bootstrap 3, 4, or 5.
https://rstudio.github.io/bslib/
Other
475 stars 57 forks source link

Clear full screen state when a full-screen card is removed from the page #1003

Closed stla closed 6 months ago

stla commented 6 months ago

Hello,

Look at the app below. Put the card in full-screen mode, then select disp in the dropdown. This causes uiOutput("content") to re-render, and then the app exits from the full-screen mode. Then there is a problem: the full-screen icon of the card does no longer appear.

library(shiny)
library(bslib)

choices <- c("cyl", "disp")

ui <- page_navbar(
  nav_panel(
    title = "Tab 1",
    fillable = FALSE,
    layout_sidebar(
      sidebar = sidebar(
        "Sidebar"
      ),
      uiOutput("content")
    )
  )
)

server <- function(input, output) {

  Title <- reactiveVal("TITLE")

  output$content <- renderUI({
    card(
      wrapper = function(...) card_body(..., height = 300, max_height = 400),
      full_screen = TRUE,
      card_header(
        tags$p(Title())
      ),
      layout_sidebar(
        sidebar = sidebar(
          selectInput(
            "select_color",
            "Marker color",
            choices = choices,
            selected = choices[1]
          )
        )
      )
    )
  })

  observeEvent(input$select_color, {
    if(input$select_color != choices[1]) 
      Title("NEW TITLE")
  })

}

shinyApp(ui = ui, server = server)

System details

Output of sessionInfo():

> sessionInfo()
R version 4.3.2 (2023-10-31 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19045)

Matrix products: default

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

time zone: Europe/Berlin
tzcode source: internal

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

other attached packages:
[1] bslib_0.6.1 shiny_1.8.0

loaded via a namespace (and not attached):
 [1] digest_0.6.34     later_1.3.2       R6_2.5.1          httpuv_1.6.14    
 [5] fastmap_1.1.1     magrittr_2.0.3    cachem_1.0.8      memoise_2.0.1    
 [9] htmltools_0.5.7   lifecycle_1.0.4   promises_1.2.1    cli_3.6.2        
[13] xtable_1.8-4      sass_0.4.8        jquerylib_0.1.4   compiler_4.3.2   
[17] rstudioapi_0.15.0 tools_4.3.2       mime_0.12         ellipsis_0.3.2   
[21] Rcpp_1.0.12       jsonlite_1.8.8    rlang_1.1.3 
cpsievert commented 6 months ago

You have a bit of a circular dependency with the output$content and input$select_color going on, which is causing this issue. If you want to update header based on the sidebar inputs, you could adopt a pattern like this one?

library(shiny)
library(bslib)

choices <- c("cyl", "disp")

ui <- page_navbar(
  sidebar = "Sidebar",
  fillable = FALSE,
  nav_panel(
    "Tab 1",
    card(
      wrapper = function(...) card_body(..., height = 300, max_height = 400),
      full_screen = TRUE,
      card_header(
        uiOutput("card_header")
      ),
      layout_sidebar(
        sidebar = selectInput(
          "select_color",
          "Marker color",
          choices = choices,
          selected = choices[1]
        ),
        "Main"
      )
    )
  )
)

server <- function(input, output) {

  Title <- reactiveVal("TITLE")

  output$card_header <- renderUI({
    Title()
  })

  observeEvent(input$select_color, {
    if(input$select_color != choices[1]) 
      Title("NEW TITLE")
  })

}

shinyApp(ui = ui, server = server)
stla commented 6 months ago

Yes, I know there's something bad, but this is just to give an (artifical) example. I didn't find a less artifical one (something more concrete which also causes the full screen to exit). Maybe any example showing this issue is artificial, I don't know actually.

cpsievert commented 6 months ago

I suppose if card() gained an ability to go full-screen when rendered, it could help with your issue, but I'd still consider it an anti-pattern compared to the solution I offered. If someone wants to provide a clearer motivation for that as a feature, I'd be happy to give it more consideration

gadenbuie commented 6 months ago

The bug here is that we're not exiting full screen mode when a full-screen card is removed from the page. In the reprex app, you can actually press Escape after the new card is added to clear the full screen state because the keyboard event listeners are still enabled.

1005 will fix this by making sure we clean up full screen state when a full-screen card is removed from the page. I don't think we should do more than that -- if you want a full-screen card to stay full-screen when updated, you'll need to use more targeted outputs as @cpsievert suggested.

github-actions[bot] commented 4 months ago

This issue has been automatically locked. If you have found a related problem, please open a new issue (with a reproducible example or feature request) and link to this issue. :raising_hand: Need help? Connect with us on Discord or Posit Community.