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 spurious duplicate values being sent by selectInput #2187

Merged
merged 3 commits into from Sep 18, 2018

Conversation

jcheng5
Copy link
Member

@jcheng5 jcheng5 commented Sep 17, 2018

Fixes #2162. I described the root cause in my comment.

The fix I made is to explicitly detect this condition in the selectize input binding, and suppress the event that (temporarily) sets the value to "". So the selectize input binding does cause an "NY" event to be raised when you backspace, but now it's correctly squelched by the client side dupe-detecting logic.

Testing notes

Here is a slightly improved reprex of the original issue. Without this fix, you will see "NY" printed at first; and if you click into the select box and backspace, you'll see "NY" printed again. With this fix, you'll only see "NY" printed the first time. In no case should you be able to get it to show an empty string for the value.

shinyApp(
  ui = fluidPage(
    selectInput("state", "Choose a state:",
      c("NY", "NJ", "CT")
    ),
    textOutput("result")
  ),
  server = function(input, output, session) {
    output$result <- renderText({
      print(input$state)
      paste("You chose", input$state)
    })
  }
)

This example adds a "Please choose one" prompt, which causes "" to now be a valid state. Both before and after this fix, backspacing should cause "" to be printed at the console, and "Please choose one" should appear in the select box.

shinyApp(
  ui = fluidPage(
    selectInput("state", "Choose a state:",
      c("Please choose one" = "", "NY", "NJ", "CT")
    ),
    textOutput("result")
  ),
  server = function(input, output, session) {
    output$result <- renderText({
      print(input$state)
      paste("You chose", input$state)
    })
  }
)

This example is like the first one except it uses selectizeInput instead of selectInput; the defaults are slightly different, in that selectizeInput allows you to have nothing selected. So with and without the fix, you should see backspace cause "" to be printed at the console, and clicking focus away from the select box control should leave it empty.

shinyApp(
  ui = fluidPage(
    selectizeInput("state", "Choose a state:",
      c("NY", "NJ", "CT")
    ),
    textOutput("result")
  ),
  server = function(input, output, session) {
    output$result <- renderText({
      print(input$state)
      paste("You chose", input$state)
    })
  }
)

@jcheng5 jcheng5 changed the title Fix spurious empty values being sent by selectInput Fix spurious duplicate values being sent by selectInput Sep 17, 2018
@@ -115,7 +115,14 @@ $.extend(selectInputBinding, {
$(el).trigger('change');
},
subscribe: function(el, callback) {
var thiz = this;
$(el).on('change.selectInputBinding', function(event) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you use an arrow function for the change.selectInputBinding callback then you don't need to explicitly capture this (just refer to this in arrow function).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed. I thought we couldn't use ES2015 here but you're right, we can.

@jcheng5 jcheng5 added this to the 1.2 milestone Sep 18, 2018
@jcheng5 jcheng5 requested review from schloerke and removed request for schloerke September 18, 2018 17:38
@jcheng5 jcheng5 merged commit c89d782 into master Sep 18, 2018
@jcheng5 jcheng5 deleted the joe/bugfix/selectize-nonempty branch September 18, 2018 20:28
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.

SelectInput sending changes to server when search box is empty
2 participants