Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The input$id variable of the navBar does not take into account the inserted panels (insertTab) #3806

Open
apalacio9502 opened this issue Apr 14, 2023 · 14 comments · May be fixed by #3810
Open

Comments

@apalacio9502
Copy link

apalacio9502 commented Apr 14, 2023

Hello,

Currently, if you have a navBarPage or a page_navbar and you do an insertTab or insert_nav in the following situation, the navbar's input$id variable does not take into account inserted panels.

I couldn't find a way to explain the problem without using a video.

The error was caught when the app is deployed to Posit Connect. As seen in the video at 1 second, I have a navBar which has three navs (Nav 0 is inserted into ui at initialization and the others via nav_insert) and when one of the navs is clicked it throws an alert with the name of the active nav. This was done by looking at the input$id of the navbar which in this case is called input$test and sending a message to js via session$sendCustomMessage.

At 11 seconds I go to another web page and return to the app with the browser arrows before Posit Connect kills the process. When I go back to the app I see that clicking on all the navs returns the name.

The problem: At 21 seconds I go to another web page and wait for Posit Connect to finish the process. In the second 47 I return to the application with the arrows of the browser. When I go back to the app I see that clicking on nav 1 and nav 2 does not return the name.

Test.Insert.Navs.mp4

Here is the application code:

library(shiny)
library(bslib)

ui <- page_navbar(
  id = "test",
  title = "Test Insert Navs",
  header = tagList(
    htmltools::tags$script(
      "Shiny.addCustomMessageHandler('active_nav', function(data) {",
      "alert(data.value);",
      "});",
    )
  ),
  nav(
    "Nav 0"
  )
)

server <- function(input, output,session) {
  
  nav_insert(id = "test", nav = nav(
    "Nav 1"
  ), select = FALSE)
  
  nav_insert(id = "test", nav = nav(
    "Nav 2"
  ), select = FALSE)
  
  observeEvent(input$test,{
    
    session$sendCustomMessage(
      type = "active_nav",
      message = list(value = input$test)
    )
  })
}

shinyApp(ui = ui, server = server)

Next I leave the information of the computer from where the application was deployed.

R version 4.2.3 (2023-03-15 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19045)

Matrix products: default

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

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

other attached packages:
[1] bslib_0.4.2 shiny_1.7.4

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.10          jquerylib_0.1.4      compiler_4.2.3       later_1.3.0         
 [5] shinyjs_2.1.0        tools_4.2.3          odbc_1.3.4           digest_0.6.31       
 [9] bit_4.0.5            memoise_2.0.1        jsonlite_1.8.4       lifecycle_1.0.3     
[13] lattice_0.21-8       pkgconfig_2.0.3      rlang_1.1.0          DBI_1.1.3           
[17] cli_3.6.1            rstudioapi_0.14      fastmap_1.1.1        withr_2.5.0         
[21] fs_1.6.1             vctrs_0.6.1          htmlwidgets_1.6.2    sass_0.4.5          
[25] hms_1.1.3            bit64_4.0.5          grid_4.2.3           fontawesome_0.5.0   
[29] R6_2.5.1             pool_1.0.1           blob_1.2.4           magrittr_2.0.3      
[33] promises_1.2.0.1     htmltools_0.5.5      ellipsis_0.3.2       rsconnect_0.8.29    
[37] mime_0.12            xtable_1.8-4         httpuv_1.6.9         shinybusy_0.3.1.9000
[41] cachem_1.0.7         zoo_1.8-12    

Regards,

@apalacio9502
Copy link
Author

@cpsievert I think the problem could be related to bslib considering that the navBar methods were migrated to bslib

@apalacio9502
Copy link
Author

Hello,

I tested deploying this same app to shinyapps.io and the behavior is similar. In this case to see the problem it is only necessary:

  1. Enter the application
  2. Go to another website
  3. Wait about 30 seconds
  4. Return to the application with the arrows of the browser.
  5. And test if clicking on Nav 1 or Nav 2 generates the alert.

On the other hand, the problem does not only happen with the input$id of the navBar:

  • If within any of the navs inserted through nav_insert there is a tabsetPanel, its input$id does not return the names of the active panel either.
  • If within any of the navs inserted through nav_insert there is a pickerInput, it does not work. Unless in the ui at initialization time dependencies are loaded via shinyWidgets:::html_dependency_picker() @pvictor

Regards,

@apalacio9502
Copy link
Author

Hello,

I think the problem is bigger than the input$id. For example, if I insert a navs_tab_card inside one of the navs inserted through nav_insert I see that the html is different.

In the following image the html of the initial app on the left and on the right the html of the app after going to another page and coming back.

image

Regards,

@gadenbuie
Copy link
Member

gadenbuie commented Apr 18, 2023

Hi @apalacio9502 and thanks for the bug report and for including a reprex. We were able to reproduce the issue you described.


We've tracked the root issue down to the interaction of two things:

  1. The Shiny app UI is cached in the browser, so when users return to an app with a running job on shinyapps but long enough that a new R process is started, the page resource is pulled from the browser cache and the UI is not re-rendered on the server.

  2. When the UI isn't re-rendered in the R process that's serving the Shiny app, we don't have access to global theme information in the server logic. This drives the behavior seen in this issue: the server doesn't know that the UI is expecting BS4+ markup for the nav items and instead emits BS3 markup, and the mismatch results in broken client logic.

There aren't easy user-facing workarounds. We need the global Shiny option bootstrapTheme to be available, but as far as I can tell, that currently also requires that we render the UI tags in the same R process where the server evaluation happens.

@schloerke and I explored a few options and we determined that Shiny could globally set Cache-Control: no-cache headers for the UI http responses. This would ensure that browsers wouldn't cache the UI page, ensuring that each new R process has theme set as expected. On the other hand, the downside there is that the UI request is always rebuilt and served again. This option is relatively easy to implement and would work immediately everywhere. (Here's a preview of that approach.)

Another option might be to serialize the theme (or key parts of it) and include in the UI response. We'd then need this information to return to the server on app start so it can be present when executing server() for the first time. I'm not sure how feasible that would be.

@schloerke schloerke linked a pull request Apr 18, 2023 that will close this issue
2 tasks
@apalacio9502
Copy link
Author

Hello @gadenbuie,

Thank you for your response!!!

I have been testing the version 596165e and the problem is resolved.

However, I would like to know if there is any way to implement the same solution with the production version of Shiny temporarily. Or if it is possible to make the same configuration to Posit Connect through https://rsc.radixu.com/__docs__/admin/appendix/configuration/#Server.CustomHeader. This problem is becoming very relevant in our case and we believe it is risky to install a development version that has significant changes according to the news.

Regards,

@jcheng5
Copy link
Member

jcheng5 commented Apr 18, 2023

@apalacio9502 I tried adding a <meta http-equiv="Cache-Control" content="no-cache"> tag and it didn't help, but this rather more drastic attempt did:

tags$head(tags$script(HTML(
  'if (performance && performance.getEntriesByType("navigation")[0].type === "back_forward") window.location.reload();'
)))

Note that this only prevents the cached version from being used in the case of the forward/back button being used, but that's also the only way I could get Chrome to use the cached version.

@gadenbuie
Copy link
Member

Hi @apalacio9502. Unfortunately, I don't know of an easy way to do this without the kind of modification we're proposing in #3810 or another method that we would have to set up. (Joe added an example as I was writing this.)

Here are two ideas that might help:

  • You can avoid this issue completely by going back to shiny::navbarPage() or to specifically use Bootstrap 3 with page_navbar(theme = bs_theme(version = 3)).
  • You could find a way to avoid using nav_insert(). For example, you might be able to start with all pages present and then use nav_remove() to remove the uneeded nav pages.

@jcheng5
Copy link
Member

jcheng5 commented Apr 18, 2023

Come to think of it, adding shinyOptions(bootstrapTheme=bslib::bs_theme()) to the top of the app.R would fix it--that's probably the simplest fix for this particular one. Just doing explicitly what page_navbar() does implicitly when it's rendered.

@apalacio9502
Copy link
Author

Hello @jcheng5,

Thank you for your response!!! Yesterday, in my attempts to solve the problem, I tried disabling the cache by inserting a meta tag as you mentioned, but it didn't work for me either.

I also did tests with a similar method to the one you propose (https://stackoverflow.com/questions/43043113/how-to-force-reloading-a-page-when-using-browser-back-button), however, the behavior the user sees is strange!!!! First, the UI is rendered in cache and then the page is reloaded.

Regards,

@apalacio9502
Copy link
Author

Come to think of it, adding shinyOptions(bootstrapTheme=bslib::bs_theme()) to the top of the app.R would fix it--that's probably the simplest fix for this particular one. Just doing explicitly what page_navbar() does implicitly when it's rendered.

Thank you @jcheng5

This solution works correctly based on the tests we carried out, however I have two questions:

  1. Would shinyOptions(bootstrapTheme=bslib::bs_theme()) override the call to bs_theme inside the page_navbar ?

  2. Inside shinyOptions(bootstrapTheme=bslib::bs_theme()) should I put theme settings like colors etc.?

On the other hand, this solution would replace the change that @gadenbuie and @schloerke proposed or would be a provisional solution?

@jcheng5
Copy link
Member

jcheng5 commented Apr 19, 2023

  1. It does not override the call; I think the page_navbar one basically takes precedence, and the default theme argument is bs_theme().

Because of this, I think you should do something like this (and this should answer your second question):

theme <- bslib::bs_theme()
# go ahead and make modifications to this theme object now

shinyOptions(bootstrapTheme=theme)

ui <- page_navbar(theme=theme, ...)

I think the change that Garrick and Barret are proposing should make this workaround unnecessary, and is generally a good change anyway.

@jcheng5
Copy link
Member

jcheng5 commented Apr 19, 2023

@gadenbuie Makes me wonder if page_navbar et al should have theme = getShinyOption("bootstrapTheme", bs_theme()) as a default instead? (I'm sure Carson will have thoughts when he gets back)

@schloerke
Copy link
Collaborator

If a theme arg is added, it should should also apply to rstudio/py-shiny

@gadenbuie
Copy link
Member

Makes me wonder if page_navbar et al should have theme = getShinyOption("bootstrapTheme", bs_theme()) as a default instead?

@jcheng5 that's certainly possible, although not my favorite as a default. I wonder if we could have bslib's global theme functions call getShinyOption() and then have the page functions use (or set) the global theme by default.

I'd have to learn more about bslib's global theme infrastructure, but that might be a holistic approach. For example, I think people might be surprised to find that

bs_global_theme(version = 5, bootswatch = "minty")
page_navbar(title = "My Page")

doesn't produce a page with the minty theme unless you explicitly use the global theme

page_navbar(title = "My Page", theme = bs_global_get())

Either case, though, would require the user to know that they need to set the theme globally outside of the UI function, which might be hard to realize given the client-side symptoms (as in this issue).

If a theme arg is added, it should should also apply to rstudio/py-shiny

@schloerke All the bslib page_ functions have a theme argument, but they default to bs_theme(), i.e. a local default theme using the latest Bootstrap. But we could tie that default value to a global theme and then tie that global theme to the Shiny option...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants