-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(Dropdown): announce dropdown item position #5193
Conversation
This change lets Downshift announce the position of dropdown item in addition to its content, when it's highlighted. Fixes carbon-design-system#4373.
Deploy preview for the-carbon-components ready! Built with commit 907c2fe https://deploy-preview-5193--the-carbon-components.netlify.com |
Deploy preview for carbon-elements ready! Built with commit 37dc456 |
Deploy preview for carbon-components-react ready! Built with commit 37dc456 https://deploy-preview-5193--carbon-components-react.netlify.com |
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.
I think we'll need to go down an alternate route for this component. Most likely the markup is not correct and that is what is causing the screen reader issues. Features like announcement position in a listbox is a screen reader feature that is computed by the underlying markup. Instead of tying this feature to VO-specific text, we should address the underlying markup issues which will in turn address the parent issue 👍 |
While automatic way for screen reader to understand our markup structure may be desirable as above comment says, Downshift explicitly takes "announcer" pattern, presumably for de-touching actual keyboard focus visual keyboard focus. So going away from "announcer" pattern is not realistic, at least for now. |
Not sure I understand, could you share more? In particular where the ideal markup is unable to be implemented given Downshift and its intended usage. |
It's not about ideal markup not implementable. It's about such ideal markup won't come into play with how contents are announced by screen reader with Downshift, given Downshift takes "announcer" pattern (similar to https://www.thinkcompany.com/2017/09/introducing-the-react-a11y-announcer/). |
@asudoh got it! Don't think it clicked that Downshift was using an |
Testing both NVDA and JAWS 2019 on Windows 10 and we've got problems. I'll open separate issues for these and wont block this PR, but the menu is navigable, but no menu items or selected options are announced whatsoever. Will need to research into Downshift and see if there's any existing bugs 🤷♂ As a reminder our target is JAWS 2019 on Firefox/Chrome/Edge on Windows 10 and VoiceOver on Safari for macOS as well as (although not a requirement) NVDA on Firefox for Windows 10 and any updates adding bug fixes/improvements/enhancements for one screen reader should be tested on all, on all platforms, on all browsers 👍 . |
Closing in favor of #5373 |
This change lets Downshift announce the position of dropdown item in
addition to its content, when it's highlighted.
Fixes #4373.
Changelog
New
getA11yStatusMessage
prop to<Downshift>
, so that Downshift announce the position of dropdown item in addition to its content, when it's highlighted.Testing / Reviewing
Testing should make sure
<ComboBox>
,<Dropdown>
and<MultiSelect>
are not broken.