fix #217: correct input$sidebarCollapsed value for edge cases

[bborgesr]

Attach input$sidebarCollapsed value to the end of the sidebar transitions (instead of when the toggle button is clicked).

Repro:

library(shiny)
library(shinydashboard)

ui <- dashboardPage(
  dashboardHeader(), 
  dashboardSidebar(),
  dashboardBody(
    textOutput("res")
  )
)

server <- function(input, output, session) {
  output$res <- renderText({
    if (input$sidebarCollapsed) {
      "Sidebar is collapsed"
    } else {
      "Sidebar is expanded"
    }
  })
}

shinyApp(ui, server)

[bborgesr]

@wch This is rebased, updated and tested as much as I could think of... I think we're good to merge.

[wch]

This is a little hard to follow since it sets the data-collapsed attribute directly, whereas in the sidebarChange function, it uses inputBinding.setValue(). Looking at the code, it's not obvious that they are both controlling the same thing.

Would it be possible to use the input binding here, or is this code executed too early for that to work?

[bborgesr]

That's right. I tried the first approach (inputBinding.setValue()), but it's too early to call here. I managed to get it working by listening on the first shiny idle event (or something along those lines), but not only is that adding extra complexity unnecessarily, but also, and worse, the visual effect is weird if you're actually accessing that value. For example, in this app:

library(shiny)
library(shinydashboard)

ui <- dashboardPage(
  dashboardHeader(), 
  dashboardSidebar(),
  dashboardBody(
    textOutput("res")
  )
)

server <- function(input, output, session) {
  output$res <- renderText({
    if (input$sidebarCollapsed) {
      "Sidebar is collapsed"
    } else {
      "Sidebar is expanded"
    }
  })
}

shinyApp(ui, server)

, you'd see the string starting off as "Sidebar is expanded" and then immediately changing to "Sidebar is collapsed."

I felt like this was okay, because (aside from the fact that shinydashboard is already full of hacks for the input bindings), we also do the exact same thing here for the selected tab item... What do you think? I could add another comment in both sections about why we have to go about this in this way.