kaijagahm / ygdpDashboard

Interactive dashboard for YGDP survey data
3 stars 0 forks source link

Bugs in app state bookmarking #33

Open kaijagahm opened 3 years ago

kaijagahm commented 3 years ago

Would be good to be able to bookmark the app state and get back to it.

Used the following resources to add this feature in https://github.com/kaijagahm/ygdpDashboard/commit/cbecad0a6f0558b168cb07bfa4fb7cfc34320cd3.

https://stackoverflow.com/questions/47569992/home-button-in-header-in-r-shiny-dashboard (code model for how to include a button in the header)

https://shiny.rstudio.com/articles/bookmarking-state.html (general bookmarking how-to)

kaijagahm commented 3 years ago

Oops, apparently this didn't quite work. I re-deployed the app, and although the button works fine, the link does not successfully go back to the bookmarked state. Maybe we have too many parameters for url bookmarking to work properly?

kaijagahm commented 3 years ago

Okay, so there are two ways to do this in Shiny. 1) URL bookmarking. This is what I tried to do above. I don't know why it's failing, but it might be because there are just too many parameters and the URL gets too long for the browser to handle.

2) Server bookmarking, where the bookmarked states are stored in the app's directory. The problem with this is that the states build up and have to be deleted after a while. Two obvious problems with deleting those are that a) I or someone would have to keep deleting them, manually, and b) deleting the bookmarks mean that old bookmarks would no longer work, which is not a great outcome.

I posted in the R4ds Slack channel, as well as the RStudio Community, about this. So far no responses. If no one can figure this out, I'm going to remove the feature.

kaijagahm commented 3 years ago

Added state bookmarking. It is mostly functional, except that for some reason, it doesn't properly bookmark the right sidebar settings in interpolation mode. Documented the issue in more detail here: https://community.rstudio.com/t/shiny-bookmarked-url-state-fails-restores-main-app-page/91594.

I'm going to leave things here, since there are other issues to discuss. Will document this bug in detail in the readme.

kaijagahm commented 3 years ago

Apart from the above-described remaining bugs, bookmarking is now mostly functional, as of commit https://github.com/kaijagahm/ygdpDashboard/commit/ebd224df2c382add8d0165b0e01747473f3de87d

kaijagahm commented 3 years ago

Ok, bookmarking for this app looks like it's going to be a major pain in the butt.

A bunch of stuff I've found on the internet says that regular old bookmarking does not work well for reactive values and/or for dynamic inputs. We have lots of both of those.

The way around this seems to be to use onBookmark and onRestore. The best guide I've found is here, though it doesn't have very extensive vignettes.

I tried the following code, listing all reactiveValues and reactiveVal objects. Note that I didn't explicitly include inputs for dynamically-generated selectors yet, nor did I include reactive expressions.

Some problems that have already come up: 1) The onBookmark/onRestore combo does not seem to work at all for reactiveVal objects, only for reactiveValues. When I try to save and load reactiveVal objects, the app crashes and no URL popup box appears.

This is potentially surmountable. I would create a reactiveValues object to aggregate all of the reactiveVal objects in the entire app.

  1. By extension, I am assuming that reactive expressions will also fail. Could probably use the same tactic to restore those.

  2. I get this weird JSON warning message when I click the Bookmark button:

    Input to asJSON(keep_vec_names=TRUE) is a named vector. In a future version of jsonlite, this option will not be supported, and named vectors will be translated into arrays instead of objects. If you want JSON object output, please use a named list instead. See ?toJSON.

I googled for that warning message, and I only found it associated with unrelated things like ggplot, such as here. I did briefly mention it in this issue although I didn't provide a reprex because I didn't know how.

Note that when I remove all reactiveVal() objects from the onBookmark/onRestore pair, the app no longer crashes, and I do still get that warning. So the warning may not actually be a problem? I think I'll proceed with ignoring it for now, and see how far that gets me.

kaijagahm commented 3 years ago

Okay, so I started trying to make a reactiveValues object to store the reactive expressions and reactiveVal objects, but there's a problem. It's easy enough to go one way: store them in a reactiveValues object. But it's really not obvious how I would go about writing the code for the other side of things: restoring the values of each reactive expression and each reactiveVal object onRestore.

Well, maybe it is obvious? Maybe I could just do observeEvent(onRestore, {}) and then include some code to assign each of those values. But I'm worried that it won't actually be that simple, and I really feel like I'm in over my head here.

Another problem I have is that I think I'd have to also explicitly bookmark the dynamically-generated inputs if I want this bookmarking to work. And I don't know how to do that, since the app is set up to accept an arbitrary number of sentence inputs. How can I add all of them? Maybe some kind of lapply statement?

All of this would be a lot of work, and it would all be for the sake of having an app for which bookmarking doesn't work on Internet Explorer (because there's a character limit for URL's on that browser). All things considered, I don't think it's worth it.

For now, I'm going to disable the bookmark button in the app code and comment out the rest. Could potentially come back to this issue, but I think we can file it in the category of "too much effort to be worthwhile".

kaijagahm commented 3 years ago

As of commit https://github.com/kaijagahm/ygdpDashboard/commit/f4fccffe353bdb3ccc2ea0fc6c93fe761859de31, removed the bookmarking infrastructure because it wasn't working. Left the code in but commented it out, and marked it with XXX.

kaijagahm commented 3 years ago

Note that Dean Attali is thinking about building a bookmarking-related package that might help with the problem of shinyapps.io not supporting server bookmarking. No idea if this will come to fruition, but it's worth keeping an eye on. He first mentioned it here.

This would not fix the above-described issues with onBookmark and onRestore, and I don't think it would fix the fundamental problem with server bookmarking where the app would take up more and more space over time unless bookmarks were deleted. But it could be interesting.

jamessmythe commented 3 years ago

Hi Kaija, I'm having similar issues with bookmarking, and have spent a couple of days creating reactiveValues, storing them on bookmarking and restoring them again.

I can get the attached reprex to work, so I may have to rebuild carefully from here...!

library(shiny)
library(tidyverse)
library(shinyjs)

homeUI <- function() {

  fluidPage(

    column(12, 
           h2("You should see this when first opening the app, but not once the bookmark has been restored"),
           actionButton(
               inputId = "other_button",
               label = 'click me to go through'
             )
           )
  )

}

otherUI <- function() {

  fluidPage(

    column(12, h2("You should see this only after clicking the button, and when restoring from bookmark"),
           bookmarkButton("bookmark this state"))
  )

}

ui <- function() {

  fluidPage(

    shinyjs::useShinyjs(),

    title = "blabla",

    div(id = "home", homeUI()),

    div(id = "other", otherUI())

  )

}

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

  showOther <- reactiveValues(other = 0)

  observeEvent(input$other_button, { #action when homepage button is clicked
    showOther$other <- 1

  }, ignoreInit = TRUE)

  observe({

    if(showOther$other == 1) {

      shinyjs::show("other")
      shinyjs::hide("home")

    } else {

      shinyjs::show("home")
      shinyjs::hide("other")

    }

  })

  onBookmark(function(state){

    state$values$oth_button <- showOther$other

  })

  onRestore(function(state) {

    showOther$other <- state$values$oth_button

  })

}

shinyApp(ui, server, enableBookmarking = "url")
kaijagahm commented 3 years ago

Thanks, James! I shudder at the thought of likewise rebuilding my own dashboard, but this is a great starting point.