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(dropdowns): allow Select to receive alphanumeric selection #786

Merged
merged 5 commits into from
Jun 29, 2020

Conversation

austingreendev
Copy link
Contributor

@austingreendev austingreendev commented Jun 25, 2020

Description

This PR allows users to highlight Select values based on the W3C list pattern

Type a character: focus moves to the next item with a name that starts with the typed character.

Type multiple characters in rapid succession: focus moves to the next item with a name that starts with the string of characters typed.

Detail

Solution

I based this interaction from the Collapsible Dropdown List W3C example as well as the Reach UI Listbox example.

The type multiple characters in rapid succession feature adds some complexity to this implementation. To accomplish this I:

  • Register the innerText of each <Item> on render
    • We are unable to rely on itemToString() as this is calculated from the value, not the text provided
  • On each Select keyDown, start a timeout for any alphanumeric key
  • Keep track of keys pressed within that timeout, while resetting the timeout each key press
  • Searching through the item text value registry based off of the current index when the initial key was selected

Deployment

As this is large net-positive for a11y and user experience, I have started this work in the v7 branch. With a majority of consumers still using v7 I wanted to validate that an approach could work across both v7 and v8 before continuing. This same work will be cherry-picked (as much as possible) onto v8 after approval.

Remaining a11y Items

A common feature of the listbox pattern is allowing the alphanumeric key press to select a matching value when the dropdown is closed. This isn't currently possible with our Downshift implementation.

Due to #613 we are unable to fully render the Item elements. This makes it impossible for us to know the current Downshift index and innerText values.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • ♿ analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

Copy link
Contributor

@louisscruz louisscruz left a comment

Choose a reason for hiding this comment

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

Looks fantastic to me!

@austingreendev austingreendev force-pushed the agreen/select-alphanumeric-sort branch 2 times, most recently from 1fb93c5 to 9426a40 Compare June 26, 2020 00:23
@austingreendev austingreendev force-pushed the agreen/select-alphanumeric-sort branch from 9426a40 to 3c0bee8 Compare June 26, 2020 00:25
Copy link
Contributor

@hzhu hzhu left a comment

Choose a reason for hiding this comment

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

LGTM, this is awesome!

packages/dropdowns/src/Select/Select.tsx Outdated Show resolved Hide resolved
Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

Can you spin-up a deployment for us to check-out the results against current examples?

@austingreendev
Copy link
Contributor Author

Here is a draft publish for review: https://5ef634b66a1798021448604b--garden-preview-v1.netlify.app/dropdowns/#select-usage

I also just realized that we intercept space commands to open/close Dropdowns. Changing this feature just for Select seems odd, but end-users are limited to only searching continuous strings. Is this something you think we should enable?

Testing Update

It seems jsdom doesn't implement innerText. So this feature is not testable currently.

@jzempel
Copy link
Member

jzempel commented Jun 26, 2020

Is this something you think we should enable?

Ref http://nativeformelements.com/. It appears that a timeout is incorporated into the native <select> to distinguish between space used for open/close/select and space used for search. We should model the same behavior.

@austingreendev
Copy link
Contributor Author

@jzempel here is an updated draft deploy that allows space in searches https://5efa1b107960e8763efa58bc--garden-preview-v1.netlify.app/dropdowns/

@austingreendev
Copy link
Contributor Author

Here is another draft: https://5efa2e5dc3b0af02161b8a42--garden-preview-v1.netlify.app/dropdowns/#menu-usage

This includes:

  • Allowing space to interact with a searchable text
  • If space is pressed in isolation it will select/close the highlighted item
  • All filtering logic is now added to the Menu element as well to align with the W3C Menu Button spec

@austingreendev austingreendev merged commit 3d8e6ad into v7 Jun 29, 2020
@austingreendev austingreendev deleted the agreen/select-alphanumeric-sort branch June 29, 2020 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants