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

downshift v5 #907

Merged
merged 30 commits into from
Feb 10, 2020
Merged

downshift v5 #907

merged 30 commits into from
Feb 10, 2020

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Jan 22, 2020

Contains refactoring, test improvements, bug fixes.

BREAKING CHANGE: Removed FunctionClearKeysSoFar from state change types and TS typings. In useSelect, once the timeout for keeping the character keys in memory has expired, FunctionSetInputValue will be used. We are doing this as we are renaming keysSoFar with inputValue.

To migrate to the new change, simply check for FunctionSetInputValue with empty string as inputValue instead of checking for FunctionClearKeysSoFar in stateReducer.

BREAKING CHANGE: Both getA11yStatusMessage and getA11ySelectionMessage will be called with the same props as the getA11yStatusMessage in <Downshift>, apart from previousResultCount. In the TS typings it's now marked as optional, and all functions have the same interface definition, A11yStatusMessageOptions.

To migrate to the new changes, in useSelect and useCombobox, if you used items as parameters in any of the a11y message functions, now you should use resultCount as probably you only needed items.length from it anyway.

Also typings UseSelectA11yMessageOptions and UseComboboxA11yMessageOptions have been removed. Use A11yStatusMessageOptions instead.

Code Changes: tests have been enhanced by using user input from RTL and they now look better and cleaner. Also covered more use cases better. Code has been refactored as well, and bundle size slightly reduced.

Functional Improvement: better focus management for both useSelect and useCombobox.

Fixes #832.
Closes #892
Closes #891
Closes #873

@codecov-io
Copy link

codecov-io commented Jan 23, 2020

Codecov Report

Merging #907 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #907   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     13    +1     
  Lines         950   1019   +69     
  Branches      190    199    +9     
=====================================
+ Hits          950   1019   +69
Impacted Files Coverage Δ
src/hooks/useCombobox/reducer.js 100% <ø> (ø) ⬆️
src/downshift.js 100% <ø> (ø) ⬆️
src/hooks/useSelect/utils.js 100% <100%> (ø) ⬆️
src/hooks/utils.js 100% <100%> (ø) ⬆️
src/hooks/testUtils.js 100% <100%> (ø)
src/hooks/useSelect/index.js 100% <100%> (ø) ⬆️
src/hooks/useCombobox/testUtils.js 100% <100%> (ø) ⬆️
src/utils.js 100% <100%> (ø) ⬆️
src/hooks/useCombobox/index.js 100% <100%> (ø) ⬆️
src/hooks/useSelect/testUtils.js 100% <100%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 860da10...3ca432f. Read the comment docs.

@silviuaavram
Copy link
Collaborator Author

Unfortunately, keeping focus on the toggleButton in the case of useSelect will prevent screen readers from entering Forms Mode when navigating in the list. It's an accessibility concern we cannot have so we should leave the focus on the menu when the dropdown is open.

That being said, most changes in this PR are still relevant. To move forward, we will add the aria attributes and handlers back to the list, move the tests back, add the Menu stateChangeTypes back etc.

@silviuaavram
Copy link
Collaborator Author

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment