Storing state of the app as a bookmark #659

usr110 · 2018-03-28T14:02:15Z

Note this is work in progress
This pertains to the issue I created yesterday - see #658
Allows user to save the state of the app on the server.

I've added a bookmark button in the input panel. Please see the screenshot below:

This button opens up a pop-up window with a URL to copy.

This might not work with the older version of shiny, so you may update it to the latest version (I am using shiny_1.0.5)

In terms of shortcomings:

It doesn't not save the state properly when:
- Region changes
- Map's bounding box (so doesn't capture any localized lines)

Would appreciate your comments @nikolai-b and @Robinlovelace

… param. This is required to save state.

…the server side - not in the URL.

…stored

usr110 · 2018-03-29T11:08:54Z

sorted Region changes in 9494e03

usr110 · 2018-03-29T12:59:31Z

sorted Map's bounding box (so doesn't capture any localized lines) in 6da322f

Robinlovelace · 2018-04-03T12:21:07Z

This looks like great progress Ali, thanks. @nikolai-b let us know if this works on the test server so we can play with getting bookmarks for a range of locations / settings.

nikolai-b

Looks good, a few questions that are mainly style but overall seems pretty clean. Nice work!

nikolai-b · 2018-04-10T20:00:49Z

regions_www/m/server.R

-      } else {
-        "isle-of-wight"
+
+      if(!is.null(region$state_region) && !is.na(region$state_region)){


Why we need to do both na and null checks? I get confused which to use in R...

null when it's not initialized - ran before the initialization of the region variable, and na when there is no saved state.

what about

if (is.character(region$state_region)){ region$current <- region$state_region }

nikolai-b · 2018-04-10T20:03:48Z

regions_www/m/server.R

+
+    region$state_lng <<- state$input$map_center$lng
+    region$state_lat <<-  state$input$map_center$lat
+    region$state_mzoom <<- state$input$map_zoom


Do you need <<-? I know we use them but we need to stop.

Good point. No, I don't need it. Have now removed it. At some point, we should rid of all of them.

nikolai-b · 2018-04-10T20:06:27Z

regions_www/m/server.R

+                                      lng = region$state_lng,
+                                      lat = region$state_lat,
+                                      zoom = region$state_mzoom
+      )


Can this be done in onRestore?

No, it can't be done there. onRestore runs before any observe block is executed - since leaflet variable hasn't been initialized, this can't work.
I now have moved this block to the onRestored function which is run after all observe blocks are executed.

…locks have been run. Relevant to https://github.com/npct/pct-shiny/pull/659/files/7bb66c9b19cb49cedb3701b62e2e74ab17acd88d#diff-e742b53b5986fdf6e52477865d38ea79

usr110 · 2018-04-11T15:10:41Z

Thanks @nikolai-b for your feedback - have now replied to your comments.

nikolai-b

After having another think sadly we need to use the url store unless you can think of another way of doing it.

Sorry I was slow at replying.

nikolai-b · 2018-04-16T18:24:57Z

regions_www/m/server.R

-      } else {
-        "isle-of-wight"
+
+      if(!is.null(region$state_region) && !is.na(region$state_region)){


what about

if (is.character(region$state_region)){ region$current <- region$state_region }

nikolai-b · 2018-04-16T18:44:50Z

regions_www/m/server.R

@@ -22,6 +22,9 @@
 ## Functions
 source("pct_shiny_funs.R", local = T)

+## Enable bookmarking at the server side
+enableBookmarking(store = "server")


Right, sadly this won't work. We need to store the state in the URL. For one we have multiple servers so if you share a link from the London server and the request is handled by the Cambridge server it will fail. Also when we deploy we are likely to wipe these.

That's a fair point. The only big problem is (apart from a messy URL for a specific states), the URL keeps changing when someone uses the app and they are not even interested to save or restore the app's state. On the bright side, it's easier to manage, as the servers don't need to remember anything.
I still believe that server-side bookmarking is much neater. I can think of two solutions here:

We don't care about app's state not saved on a specific server

For the two servers, we routinely mirror their saved states. This requires writing a routine which runs periodically to manage.

What do you think?

Not sure what you mean with

the URL keeps changing when someone uses the app

I've switched to store = "url" and in my browser the URL doesn't change when I'm moving around. Does it in yours?

Does solution (1) mean we provide a bookmark system that works 50% of the time? Not sure I like that.

Solution (2) is possible but I've not got budgeted time to do it, also I think it is likely to be a hacky fix. Even per server we have multiple instances of R running which could well interact in surprising ways.

Server side results in a neater URL but a much messier code.

I've pushed fdd151b as a WIP but really I think it might be easier with the long messy URL sadly.

Nice work @nikolai-b

I've switched to store = "url" and in my browser the URL doesn't change when I'm moving around. Does it in yours?

No, it doesn't. Sorry for the confusion. It keeps changing in another app, so I thought it will behave the same here.

You've convinced me to use the url bookmarking, so thanks (as long as we can provide a tiny URL). However I am unable to test your code in fdd151b

When I debug the state variable used in the function:

pct-shiny/regions_www/m/server.R

Lines 81 to 91 in fdd151b

oldEncodeShinySaveState <- getFromNamespace("encodeShinySaveState", "shiny")

shortEncodeShinySaveState <- function(state){

res <- oldEncodeShinySaveState(state)

r <- httr::POST(

"https://www.googleapis.com/urlshortener/v1/url?key=AIzaSyBLGLnQojQm2WQu9Urri2mmVTQolrLuJBc",

body = list(longUrl = paste0("http://www.pct.bike/?", res)), encode = "json"

)

return(httr::content(r)$id)

}

assignInNamespace("encodeShinySaveState", shortEncodeShinySaveState, ns="shiny")

I see that it's NULL - see below:

Do I need to test it on the shiny-server?

nikolai-b · 2018-04-16T18:55:09Z

regions_www/m/server.R

+  # (including zoom from the saved state of the app)
+  onRestored(function(state) {
+
+    if(!is.null(region$state_region) && !is.na(region$state_region)){


I don't think this is needed. For onRestored to be called then the state_region must be set, right?

Good point, Nikolai. Have removed these checks.

Appears wrong on link, exposes our API key

Robinlovelace · 2018-04-18T22:45:37Z

regions_www/m/server.R

+shortEncodeShinySaveState <- function(state){
+  res <- oldEncodeShinySaveState(state)
+  r <- httr::POST(
+    "https://www.googleapis.com/urlshortener/v1/url?key=AIzaSyBLGLnQojQm2WQu9Urri2mmVTQolrLuJBc",


I generally suggest saving API keys in the .Renviron file, as described here: https://csgillespie.github.io/efficientR/set-up.html#renviron in this case. Is there no issue with rate limits? Maybe no issue sharing this with the world in that case... Another compromise, in this case between the + of short urls and the + of reducing dependencies. In this case I'd lean slightly towards reducing dependencies although can understand the desire to keep URLs short.

Nudge on this @nikolai-b - suggest not publicising the key in the code.

Thanks for spotting @Robinlovelace ! Very late response sorry I just saw this. You can see the commit message is

Dummy attempt at keeping URL short

Appears wrong on link, exposes our API key

This API key was locked down to only work from pct.bike and I disabled a few days after posting it here (was just so others could try it) but it just shows what we could do...
Since then the whole goo shortner has been deprecated so to be completely clear this is not the approach I'd suggest it was just to show it was possible to hack the URL to make it shorter, I think using assignInNamespace is probably a bad idea!

Never come across assignInMyNamespace. The standard way for users to handle API keys now is:

usethis::edit_r_environ() # add new line like KEY=XYZ # Restart R Sys.getenv("KEY")

Would that approach work on the server (if we ever did want to use API keys)?

Late response to a late response!

Robinlovelace · 2019-03-01T14:33:35Z

Any update on the status of this @usr110 and @nikolai-b ? Not a priority atm so I would suggest we close this and look to re-open after the schools layer (and maybe near market) features are added.

Robinlovelace · 2020-08-06T17:01:45Z

Closing for now as it's gone stale.

usr110 added 5 commits March 28, 2018 14:42

Make ui variable which has shinyUI insside a function with request as…

28f0885

… param. This is required to save state.

Add a bookmark button without any label

58606ee

Change icon of bookmark button from link to bookmark

38e8fdd

Enable bookmarking functionality which saves the state of the app on …

b7f7ec7

…the server side - not in the URL.

Save state's region in a region var and restore it when session is re…

9494e03

…stored

Store and then restore map lat and lng along with zoom for state

6da322f

usr110 requested a review from nikolai-b March 29, 2018 15:35

Merge remote-tracking branch 'origin/master' into bookmark

7bb66c9

nikolai-b reviewed Apr 10, 2018

View reviewed changes

nikolai-b mentioned this pull request Apr 10, 2018

Try packrat #643

Closed

usr110 added 2 commits April 11, 2018 15:45

Use onRestored function to set the map's location after all observe b…

ef48e28

…locks have been run. Relevant to https://github.com/npct/pct-shiny/pull/659/files/7bb66c9b19cb49cedb3701b62e2e74ab17acd88d#diff-e742b53b5986fdf6e52477865d38ea79

Move comments inside the function

95c3b84

nikolai-b reviewed Apr 16, 2018

View reviewed changes

usr110 and others added 2 commits April 18, 2018 15:45

Remove additional checks for state_region. Relevant to #659 (comment)

0dd87b6

Dummy attempt at keeping URL short

fdd151b

Appears wrong on link, exposes our API key

Robinlovelace reviewed Apr 18, 2018

View reviewed changes

Robinlovelace closed this Aug 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storing state of the app as a bookmark #659

Storing state of the app as a bookmark #659

usr110 commented Mar 28, 2018

usr110 commented Mar 29, 2018

usr110 commented Mar 29, 2018

Robinlovelace commented Apr 3, 2018 •

edited

Loading

nikolai-b left a comment

nikolai-b Apr 10, 2018

usr110 Apr 11, 2018

nikolai-b Apr 16, 2018

nikolai-b Apr 10, 2018

usr110 Apr 11, 2018

nikolai-b Apr 10, 2018

usr110 Apr 11, 2018

usr110 commented Apr 11, 2018

nikolai-b left a comment

nikolai-b Apr 16, 2018

nikolai-b Apr 16, 2018

usr110 Apr 18, 2018

nikolai-b Apr 18, 2018

usr110 Apr 23, 2018

nikolai-b Apr 16, 2018

usr110 Apr 18, 2018

Robinlovelace Apr 18, 2018

Robinlovelace Apr 23, 2018

nikolai-b Nov 18, 2018

Robinlovelace Nov 22, 2018

Robinlovelace commented Mar 1, 2019

Robinlovelace commented Aug 6, 2020

	oldEncodeShinySaveState <- getFromNamespace("encodeShinySaveState", "shiny")

	shortEncodeShinySaveState <- function(state){
	res <- oldEncodeShinySaveState(state)
	r <- httr::POST(
	"https://www.googleapis.com/urlshortener/v1/url?key=AIzaSyBLGLnQojQm2WQu9Urri2mmVTQolrLuJBc",
	body = list(longUrl = paste0("http://www.pct.bike/?", res)), encode = "json"
	)
	return(httr::content(r)$id)
	}
	assignInNamespace("encodeShinySaveState", shortEncodeShinySaveState, ns="shiny")

Storing state of the app as a bookmark #659

Storing state of the app as a bookmark #659

Conversation

usr110 commented Mar 28, 2018

usr110 commented Mar 29, 2018

usr110 commented Mar 29, 2018

Robinlovelace commented Apr 3, 2018 • edited Loading

nikolai-b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

usr110 commented Apr 11, 2018

nikolai-b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Robinlovelace commented Mar 1, 2019

Robinlovelace commented Aug 6, 2020

Robinlovelace commented Apr 3, 2018 •

edited

Loading