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

feat(hooks): add useSelect hook #747

Merged
merged 41 commits into from
Sep 10, 2019
Merged

feat(hooks): add useSelect hook #747

merged 41 commits into from
Sep 10, 2019

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Aug 18, 2019

Adding useSelect, the first hook of Downshift.

Downshift is often used to create custom <select> widgets. Even if it's designed as an autocomplete solution first, it proves it's a versatile component and we should aim to keep it this way.

As a follow-up to #683 I started the change to the hooks-based API by writing useSelect.

It aims to provide the same functionality and accessibility as a native HTML <select>. It should follow the ARIA Dropdown Pattern for consistency reasons (it should work on Windows/Mac, with Jaws/NVDA/VoiceOver/ChromeVox no matter if the browser is Chrome/Safari/Firefox/Edge/IE).

The API is meant to resemble Downshift's.

Functionality provided out of the box:

  • focus switch between the toggle button and the list. Combined with proper aria attributes on both these elements, widget should be accessible via all screen readers.
  • toggle functionality with mouse, Enter, Arrow Keys and Space.
  • selection with click, Enter and blur (by Tab, Shift-Tab and outside click).
  • highlight option by typing character keys.
  • a11y aria-live message on menu open and item select.
  • highlight with Arrow keys that can be combined with Shift, End and Home.

@silviuaavram silviuaavram changed the title Feat/use select hook feat(hooks): add useSelect hook Aug 18, 2019
@codecov-io
Copy link

codecov-io commented Aug 21, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #747    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files           3      9     +6     
  Lines         418    694   +276     
  Branches      105    153    +48     
======================================
+ Hits          418    694   +276
Impacted Files Coverage Δ
src/hooks/useSelect/stateChangeTypes.js 100% <100%> (ø)
src/hooks/useSelect/testUtils.js 100% <100%> (ø)
src/hooks/useSelect/utils.js 100% <100%> (ø)
src/hooks/utils.js 100% <100%> (ø)
src/hooks/useSelect/index.js 100% <100%> (ø)
src/hooks/useSelect/reducer.js 100% <100%> (ø)
... and 2 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 6ae0bbc...635345e. Read the comment docs.

@silviuaavram silviuaavram merged commit 21199b9 into master Sep 10, 2019
@silviuaavram silviuaavram deleted the feat/useSelect-hook branch September 10, 2019 17:04
@silviuaavram
Copy link
Collaborator Author

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@@ -8,6 +8,18 @@ config.plugins[cjsPluginIndex] = commonjs({
include: 'node_modules/**',
namedExports: {
'react-is': ['isForwardRef'],
react: ['useReducer', 'useEffect', 'useRef', 'useState'],
'node_modules/keyboard-key/src/keyboardKey.js': ['getKey'],

Choose a reason for hiding this comment

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

Why was this added? Wouldn't this require all rollup clients to include this?

}

export interface UseSelectProps<Item> {
items: Item[]
Copy link

@Daniel15 Daniel15 Dec 18, 2019

Choose a reason for hiding this comment

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

I think this could actually be readonly Item[] (or ReadonlyArray<Item> which is equivalent) as Downshift doesn't mutate this array, right? What do you think about changing it?

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

Successfully merging this pull request may close these issues.

4 participants