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

Fix updateDateRangeInput when only one of start/end updated #1315

Merged
merged 1 commit into from Aug 23, 2016

Conversation

gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Aug 23, 2016

App to test:

shinyApp(
  ui = fluidPage(
    div(
      dateRangeInput("daterange", "Date range:",
                     start = "2001-01-01",
                     end   = "2010-12-31")
    ),
    actionButton("btn1", "leave start & end alone"),
    actionButton("btn2", "set date range start"),
    actionButton("btn3", "set date range end"),
    actionButton("btn4", "set date range start and end")
  ),
  server = function(input, output, session) {
    observeEvent(input$btn1, updateDateRangeInput(session, "daterange"))
    observeEvent(input$btn2, updateDateRangeInput(session, "daterange", start = "2005-01-01"))    
    observeEvent(input$btn3, updateDateRangeInput(session, "daterange", end = "2006-01-01"))
    observeEvent(input$btn4, updateDateRangeInput(session, "daterange", start = "2007-01-01", end = "2008-01-01"))
  }
)

@wch wch merged commit 9dbe434 into rstudio:master Aug 23, 2016
@wch
Copy link
Collaborator

wch commented Aug 23, 2016

I think this isn't quite right. When click the middle two buttons, it sets one value, but blanks out the other. Isn't that supposed to happen only when the value is NA?

@gaborcsardi
Copy link
Contributor Author

Hmmm. Interesting. I tested it before the other PR was merged, will test it again in a sec.

@gaborcsardi
Copy link
Contributor Author

It looks like the R list(NULL, NULL) is converted to [null, null] in JS, just like list(NA, NA).
:( This was not a problem before the other PR, because null values were ignored....

E.g. see the first two buttons.

shinyApp(
  ui = fluidPage(
    div(
      dateRangeInput("daterange", "Date range:",
                     start = "2001-01-01",
                     end   = "2010-12-31")
    ),
    actionButton("btn1", "leave start & end alone"),
    actionButton("btn11", "reset start & end with NA"),
    actionButton("btn2", "set date range start"),
    actionButton("btn3", "set date range end"),
    actionButton("btn4", "set date range start and end")
  ),
  server = function(input, output, session) {
    observeEvent(input$btn1, updateDateRangeInput(session, "daterange"))
    observeEvent(input$btn11, updateDateRangeInput(session, "daterange", start = NA, end = NA))
    observeEvent(input$btn2, updateDateRangeInput(session, "daterange", start = "2005-01-01"))    
    observeEvent(input$btn3, updateDateRangeInput(session, "daterange", end = "2006-01-01"))
    observeEvent(input$btn4, updateDateRangeInput(session, "daterange", start = "2007-01-01", end = "2008-01-01"))
  }
)

How about passing a named list to JS? That should solve this I think.

@wch
Copy link
Collaborator

wch commented Aug 23, 2016

A named list sounds good idea.

gaborcsardi added a commit to MangoTheCat/shiny that referenced this pull request Aug 23, 2016
@gaborcsardi
Copy link
Contributor Author

gaborcsardi commented Aug 23, 2016

See #1317. A named list is passed now, but NULL values are dropped from it, so they have to effect.

wch added a commit that referenced this pull request Aug 24, 2016
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 this pull request may close these issues.

None yet

2 participants