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

Combobox with list and both autocomplete: Fix issue with listbox not updating #1335

Merged
merged 3 commits into from
Feb 28, 2020

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Feb 16, 2020

Fixes #1332.

Preview link for fix for listbox not properly updating using backspace (Issue 1332)

I added calls to the code that filters the options after the filter property is updated when backspace is pressed or if the length of the textbox value is less than the length of the filter property.

Review checklist

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

This is much nicer - thanks @jongund !
+1

@mcking65 mcking65 self-requested a review February 17, 2020 00:23
Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

@jongund, thank you for the awesome, fast turn-around on this!! Functionally, this looks great! It passes the functional tests I specified in the issue.

@mcking65
Copy link
Contributor

@spectranaut, @smhigley, seems like it would be a good idea to have tests for this behavior as specified in issue #1332 and associated with data-test-id="standard-single-line-editing-keys". What do you think?

@mcking65 mcking65 changed the title Issue1332 listbox not updating Combobox with list and both autocomplete: Fix issue with listbox not updating Feb 17, 2020
@mcking65
Copy link
Contributor

@smhigley, are you available for code and test review? If we are going to change/add tests, we could break that out into a separate issue and PR to keep this one simple.

@smhigley
Copy link
Contributor

@mcking65 I agree, I think a test should probably be added for backspace correctly updating the filter, but that can be a separate issue/PR

@mcking65
Copy link
Contributor

Created #1345 to add regression testing for backspace.

@mcking65 mcking65 merged commit b543058 into master Feb 28, 2020
@mcking65 mcking65 deleted the issue1332-listbox-not-updating branch February 28, 2020 21:52
michael-n-cooper pushed a commit that referenced this pull request Feb 28, 2020
Combobox with list and both autocomplete: Fix issue with listbox not updating (pull #1335)

* fixed #1332 by adding call to filterOptions after updating filter
* added update filterOptions when ever backspace is pressed

Co-authored-by: Matt King <a11yThinker@Gmail.com>
carmacleod pushed a commit to carmacleod/aria-practices that referenced this pull request Feb 29, 2020
…updating (pull w3c#1335)

* fixed w3c#1332 by adding call to filterOptions after updating filter
* added update filterOptions when ever backspace is pressed

Co-authored-by: Matt King <a11yThinker@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants