[WIP] Pesudo-navigation

[bborgesr]

Requirements

These are the main aspects I considered for this feature:

1. We need to give a way for the app author to modify the query string. This is accomplished by the new pushState() function in Shiny. (Side note: for now, this maintains the same three arguments of the pushState JS function. But since I don't think we have a use case for which the first two arguments should be anything other than NULL, I think we can just ask only for the query string). For example, to change the query string based on a selectInput named input$employees, we can do the following:

   observe({
       pushState(NULL, NULL, paste0("?employee=", input$employees))
    })

2. The current query string (modified arbitrarily many times using the pushState function) should be "queryable" in any reactive context (from a user perspective, this should look like just another input$ type object). Currently, I'm not sugarcoating much, so to get the query (in the format of key = value), we do something like:

   query <- parseQueryString(session$clientData$url_search)

   I think it would be worth it to create a wrapper called getQueryString such that we could do something like:

   query <- getQueryString()

3. The same content should be displayed if we direct-link to an app that already has a query string. This is possible, provided you have the right server design (see example at the bottom).

HTML5 History API peculiarities

The new history API basically consists of two functions:

1. The function history.pushState(state, title, url_to_append); which pushes a new history entry onto the history stack and which changes the url.
2. The event listener window.onpopstate = function(e) {}; which is fired whenever there is "a browser action such as clicking on the back button (or calling history.back() in JavaScript)" (from Mozilla). It pops an entry off of the history stack.

What I find really weird about this API is that the description reads:

A popstate event is dispatched to the window every time the active history entry changes between two history entries for the same document.

However, a few lines below we learn that:

Note that just calling history.pushState() or history.replaceState() won't trigger a popstate event.

Which is weird, because calling history.pushState() is definitely a change in the active history entry changes between two history entries...

The reason we need onpopstate is because that is the ideal moment to update the session$clientData$url_search param (unfortunately calling history.pushState() does generate any event of its own - like changing the hash did with onhashchange - so loading of new content cannot be detected anymore.

If onpopstate was also fired when a call is made to history.pushState() (as well the regular clicks on the back and forward buttons), then, we could do (in the source JS):

addMessageHandler('pushState', function(message) {
    window.history.pushState(message.state, message.title, message.url);
});

window.onpopstate = function(e) {
    inputs.setInput('.clientdata_url_search', window.location.search);
};

But this is not the case. To get around the problem, I did the easiest, most reliable hack -- just call history.pushState() twice, so I can immediately call history.back() (which is what effectively triggers the onpopstate event listener):

addMessageHandler('pushState', function(message) {
    window.history.pushState(message.state, message.title, message.url);
    window.history.pushState(message.state, message.title, message.url);
    window.history.back();
});

It's ugly, but it works smoothly.

But I'm not the first one to be frustrated with this problem. This SO answer suggests a monkey-patch around history.pushState():

(function(history){
    var pushState = history.pushState;
    history.pushState = function(state) {
        if (typeof history.onpushstate == "function") {
            history.onpushstate({state: state});
        }
        // ... whatever else you want to do
        // maybe call onhashchange e.handler
        return pushState.apply(history, arguments);
    }
})(window.history);

// and then...
window.onpopstate = history.onpushstate = function(e) { ... }

The idea is that:

So whenever the pushState method is called, you can trigger your own event handler.
It is a hack, but until an event is raised by default, this seems to be the only way to get notified when history.pushState() is called.

I couldn't get this to work quite right (the app updates in the right order, but always one step behind). Even if this did work, I'd feel a lot less comfortable with this option than the former, since it seems to be browser-dependent (it didn't work in any browser for me, but it was at least meant only as a Firefox workaround).

Example app

In order to see the "hack version" (i.e. calling history.pushState() twice and history.back() once), install:

## hack
devtools::install_github("rstudio/shiny", ref="96be6ed6508fc03cda3e20cface7d13b49a57b24")

In order to see the "monkey-patch version" (which, again, does not work quite right), install:

## monkey patch
devtools::install_github("rstudio/shiny", ref="c3167459e293f6b2ce8139984c5ddcb2cf096520")

Then, try the app below. It works by changing the query string depending on the selectInput choice made by the user; then, the query string is used to choose what to display in a uiOutput block. If you direct link already with a valid query string (ex: 127.0.0.1:9000/?employee=Joe), it will show you the correct content.

library(shiny)

ui <- fluidPage(
  titlePanel("Pseudo-Navigation Demo"),
  sidebarLayout(
    sidebarPanel(
      selectInput("employees", "Choose an employee:", 
                  c(Choose = "", "Barbara", "Winston", "Joe"))
    ),
    mainPanel(
      uiOutput("profile")
    )
  )
)

server <- function(input, output, session){
  observe({
    req(input$employees)
    pushState(NULL, NULL, paste0("?employee=", input$employees))
  })

  output$profile <- renderUI({
    query <- parseQueryString(session$clientData$url_search)

    if (query$employee == "Barbara") {
      return("This is Barbara")
    } else if (query$employee == "Winston") {
      return("This is Winston")
    } else if (query$employee == "Joe") {
      return("This is Joe")
    } else {
      return("Not an employee")
    }
  })
}

shinyApp(ui, server)

Decisions

1. Do we change pushState(state, title, url) to something sugarcoated like updateQueryString(queryString)?
2. Do we change parseQueryString(session$clientData$url_search) to getQueryString()?
3. Do we use my hack to trigger onpopstate, or we figure out a better way of doing this?
4. Am I missing anything?
   (well, except for the documentation - I haven't spent any time on that yet because I wasn't sure what the final solution would look like)

cc @jcheng5 @wch

[jcheng5]

This is too early. The original history.pushState hasn't been called yet, so the URL hasn't yet changed. That's the cause of your behind-by-one bug.

[bborgesr]

Hmm, I think I don't fully follow you on this. I thought your comment meant that I should move it further into the app, so I did in commit 71a63119f05ee055369ee2f9ee3fb21e1f80d724 - also making sure that history.pushState has been called and the URL has changed. But we still have the same problem. Which makes me think I haven't really understood your comment.

[bborgesr]

Also, does this mean you for the "monkey-path" version, instead of the "hack" version (given that the latter doesn't need this function at all)?

[jcheng5]

If possible I'd prefer not onpopstate/onpushstate, but addEventListener("popstate", ...) (or preferably, the jQuery equivalent). I don't know if there's something specific about pop/pushstate but in general you want to avoid the attribute-assignment style of event subscribing, since you could be overwriting a previous handler and are also susceptible to someone later overwriting yours.

[jcheng5]

I can't decide whether I like how closely this follows the HTML5 API. With updateQueryString we don't assume you know the HTML5 jargon. For sure, pushState is far too general a function name--keep in mind that in HTML5 you're accessing it via the history object, i.e. history.pushState() is far less vague than just pushState().

Maybe navigateQueryString(queryString, session)?

[bborgesr]

Yes, totally agree. Although I think it will be confusing for users to have two functions (updateQueryString and navigateQueryString) that do exactly the same from an user perspective (change the query string)... As I look more into it, we don't really need to use replaceState() for bookmarking. If the goal is just to update the query string, we can achieve the same effect by calling pushState() (which does change the query string just the same, but also adds a new history entry onto the stack -- fortunately this is irrelevant for bookmarkable state). So I propose we get rid of the pushState function in Shiny altogether and use updateQueryString() for both scenarios, given that we change its implementation from:

addMessageHandler('updateQueryString', function(message) {
    window.history.replaceState(null, null, message.queryString);
});

to:

addMessageHandler('updateQueryString', function(message) {
    window.history.pushState(null, null, message.queryString);
});

We'd also have to add examples of the new usage (i.e. keep the bookmarking example copied below, but also add at least one example of how to use this for pseudo-navigation).

To make my point that it is indifferent to use replaceState or pushState when dealing with bookmarking, try the app below and toggle which line is used (updateQueryString(url) or pushState(NULL, NULL, url)) -- you'll see it works exactly the same for bookmarking purposes:

ui <- function(req) {
  fluidPage(
    textInput("txt", "Text"),
    checkboxInput("chk", "Checkbox")
  )
}
server <- function(input, output, session) {
  observe({
    reactiveValuesToList(input)
    session$doBookmark()
  })
  onBookmarked(function(url) {
    # ========= TOGGLE COMMENTED OUT LINES ========== #
    # pushState(NULL, NULL, url)
    # updateQueryString(url)
  })
}
enableBookmarking("url")
shinyApp(ui, server)

The only problem with this is that you wouldn't be able to use updateQueryString() for both purposes simultaneously (which would be a pretty complicated mess even if we let the two separate functions be separate: replaceState for updateQueryString and pushState for navigateQueryString). Note that this doesn't mean you can't do bookmarking and navigation simultaneously -- it's only the very specific type of "live" bookmarking that you can't do at the same time as navigation (which, I think, makes sense that is not simultaneously supported since when you "navigate", you're presumably walking away from inputs on that "page" anyway). The only reason that there would be a good scenario to support both features simultaneously is if people wanted to have multiple-page, navigable apps and also have all inputs across all subpages to be bookmarked in the same url. Even then, I think any practical implementation of this would look like a mess and what would really preferable in those situations would be to use the "Save to server" option for bookmarking and leave the query string to handle only navigation (except, of course, for the _state_id_ param). This is already possible with this implementation given that either:

1. it's trivial to get the correct subpage from the inputs alone (like the example app in the description of this PR);
2. the code to get the correct subpage in included in the onRestore() function.

So, I don't think we're really losing any substantial functionality by doing this...

In any case, if this use case is frequent enough, I still think we should not address it here and now, because the only real way to address it would be to support proper navigation (i.e. have app/subpage rather than app/?param=value).

Aside: This whole exposition of the problem made me realize that when we do implement proper navigation, we should use modules under the hood. The namespace would be the relative url of each subpage. This would allow subpages to use the same ids for inputs and outputs and still preserve the uniqueness of ids for the final app. Even though it looks like a minor advantage, I think it could make app author's lives easier by literally using the same template for similar subpages (in the case of my example app, this would be each employee's profile).

@wch, @jcheng5, what do you think?

[wch]

I think this should be .on instead of .bind. See: http://api.jquery.com/bind/

[bborgesr]

@wch, @jcheng5 I think this is finally ready for merging. I went through most things with Joe last week, but if there are any doubts about anything, we should probably schedule some time to go over it, as there is quite bit here and there was a fair amount of decisions involved.

When we're ready to merge, I can either rebase here or we can squash merge.

Here is the initial sample app, rewritten with the final (?) code:

library(shiny)

ui <- fluidPage(
    titlePanel("Pseudo-Navigation Demo"),
    sidebarLayout(
        sidebarPanel(
            selectInput("employees", "Choose an employee:", 
                        c(Choose = "", "Barbara", "Winston", "Joe"))
        ),
        mainPanel(
            uiOutput("profile")
        )
    )
)

server <- function(input, output, session){
    observe({
        req(input$employees)
        updateQueryString(paste0("?employee=", input$employees),
                          mode = "push")
    })
    
    output$profile <- renderUI({
        query <- getQueryString()
        
        if (query$employee == "Barbara") {
            return("This is Barbara")
        } else if (query$employee == "Winston") {
            return("This is Winston")
        } else if (query$employee == "Joe") {
            return("This is Joe")
        } else {
            return("Not an employee")
        }
    })
}

shinyApp(ui, server)

[bborgesr]

Closes #1418