-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 ListBox typeahead spacebar issue #869
Conversation
a169e36
to
39d7de0
Compare
@@ -489,6 +493,34 @@ describe('ListBox', function () { | |||
expect(document.activeElement).toBe(options[4]); | |||
}); | |||
|
|||
it('supports the space character in a search', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you use jest.useFakeTimers, you can step through time to verify that you can use space bar to select the option after the typeahead timeout has completed, i won't require that you write that test, just an interesting thing you can do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @snowystinger, good idea! I called it out in the test instructions, so I think I should include it as well as a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -88,6 +99,8 @@ function getStringForKey(key: string) { | |||
// See https://www.w3.org/TR/uievents-key/ | |||
if (key.length === 1 || !/^[A-Z]/i.test(key)) { | |||
return key; | |||
} else if (key === 'Spacebar') { // For IE 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't care about IE 11 anymore if you want to remove this, no sense in dead code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Closes #851
✅ Pull Request Checklist:
📝 Test Instructions:
Section 8
🧢 Your Project:
Adobe React-Spectrum