Fix: a11y, enabled clear indicator to be accessible via keyboard

JedWatson / react-select

The Select Component for React.js

https://react-select.com/

MIT License

27.44k stars 4.1k forks source link

Fix: a11y, enabled clear indicator to be accessible via keyboard #5850

Open yhy-1 opened 5 months ago

yhy-1 commented 5 months ago

As discussed in https://github.com/JedWatson/react-select/issues/4988

note: Since focus needs to be set back to the select input, it announces that the "combo box has auto-complete select blank" instead of what is in the aria-live since focus takes precedence by the screen reader.

changeset-bot[bot] commented 5 months ago

🦋 Changeset detected

Latest commit: ced2a2a8dfb25fd714334ce5ecb04f58442f2b05

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ------------ | ----- | | react-select | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci[bot] commented 5 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ced2a2a8dfb25fd714334ce5ecb04f58442f2b05:

Sandbox	Source
react-codesandboxer-example	Configuration

yhy-1 commented 5 months ago

@lukebennett88 @JedWatson @dcousens @nderkim @Rall3n @Methuselah96 , would you mind reviewing this PR.

yhy-1 commented 5 months ago

@lukebennett88 @JedWatson @dcousens @nderkim @Rall3n @Methuselah96

Hi, would appreciate it if you have time to review this PR.

mellis481 commented 3 months ago

@lukebennett88 @JedWatson @dcousens @nderkim @Rall3n @Methuselah96 Can you please review/approve this PR? It's a critical accessibility feature that countless teams need.

jossmac commented 3 months ago

I'm not an a11y expert, but I have some thoughts:

accessible patterns should be the default, rather than making consumers opt-in e.g. enableAccessibleClearIndicator
using hard-coded english for labels will become a problem e.g. aria-label="clear"
my initial instinct is that ESC press, when the dialog is closed, should clear the value. i feel like the clear button would introduce an unexpected tab stop; like, a combobox doesn't have a focusable button for invoking the menu, you just press DOWN|UP

EDIT: Turns out this is already implemented via DEL press. I think a better solution would be to add that information to the initial announcement (when isClearable).

dcousens commented 3 months ago

@yhy-1 as suggested by @jossmac, could you update your pull request to announce the DEL shortcut in the aria-live attribute? (see https://github.com/JedWatson/react-select/blob/06e34882638d1526b9f5a1238bb567a3e9460ce5/packages/react-select/src/components/LiveRegion.tsx)

dcousens commented 3 months ago

Sorry @yhy-1 that I missed your previous pings/notifications :yellow_heart:

yhy-1 commented 2 months ago

@jossmac @dcousens Hi, there isn't a consensus on the accessible patterns for this clear button, that is why it was made optional props in this PR. You can read in the discussion at https://github.com/JedWatson/react-select/issues/4988. Our company had flagged this as a usability issue. We want the ability to enable this to be keyboard accessible but have no means to do so. I can update the hard-code aria-label to add the ability for an aria-label. Would this PR be accepted? The aria-live already consists of so much stuff, that it would eventually get lost and won't meet the requirement for our company.

jossmac commented 2 months ago

Thank you for detailing your requirements and constraints, providing context helps a great deal. However, I'm still not comfortable introducing an unexpected tab stop.

there isn't a consensus on the accessible patterns for this clear button

After a little research, there's a keyboard interaction spec on this behaviour for the combobox pattern:

Escape: Dismisses the popup if it is visible. Optionally, if the popup is hidden before Escape is pressed, clears the combobox.

@dcousens we should support the spec behaviour*. @yhy-1 This would meet the requirement for your company, right?

*Maybe in addition to Delete press? We'd need to prevent default on the Escape keyboard event, so consumers can respond accordingly when implementing modal dialog close behaviour etc.

mellis481 commented 2 weeks ago

@jossmac It looks like currently:

DELETE and BACKSPACE will clear input if the popup is not visible.
DELETE and BACKSPACE will not clear input if the popup is visible.
ESCAPE has no effect on clearing input.

I agree that react-select should support the clearing of input when ESCAPE is pressed when the popup is not visible.