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

CustomSelectControl: read currently selected value on focus #21461

Closed

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Apr 7, 2020

Fixes #21459.

Description

In WooCommerce Blocks we are using the CustomSelectControl component to allow the user to select a Country. We noticed when focusing it, the currently selected value is not presented to the user if they use a screen reader. (See woocommerce/woocommerce-blocks#2040).

How has this been tested?

I tested the CustomSelectControl component in the story book and in the Preset Size option of the Typography panel of the paragraph block. Basically testing that navigating to it with a screen reader presents the selected option to the user (ie: Large currently selected).

Types of changes

a11y improvement

Checklist:

  • My code follows the WordPress code style.
  • My code follows the accessibility standards.

@Aljullu Aljullu added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] UI Components Impacts or related to the UI component system labels Apr 7, 2020
@Aljullu Aljullu self-assigned this Apr 7, 2020
@github-actions
Copy link

github-actions bot commented Apr 7, 2020

Size Change: +77 B (0%)

Total Size: 835 kB

Filename Size Change
build/block-library/index.js 112 kB +3 B (0%)
build/components/index.js 198 kB +70 B (0%)
build/core-data/index.js 11.4 kB +1 B
build/data/index.js 8.43 kB +1 B
build/edit-post/index.js 27.7 kB +1 B
build/editor/index.js 43.3 kB +1 B
build/rich-text/index.js 14.8 kB +2 B (0%)
build/shortcode/index.js 1.69 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.05 kB 0 B
build/block-library/editor.css 7.05 kB 0 B
build/block-library/style-rtl.css 7.14 kB 0 B
build/block-library/style.css 7.14 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 11 kB 0 B
build/edit-site/style-rtl.css 5.26 kB 0 B
build/edit-site/style.css 5.25 kB 0 B
build/edit-widgets/index.js 8.33 kB 0 B
build/edit-widgets/style-rtl.css 5 kB 0 B
build/edit-widgets/style.css 5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/style-rtl.css 3.27 kB 0 B
build/editor/style.css 3.27 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.32 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad youknowriad added the Needs Accessibility Feedback Need input from accessibility label Apr 8, 2020
@Aljullu Aljullu force-pushed the fix/21459-customselectcontrol-read-value-on-focus branch from dd28227 to 275f85b Compare April 24, 2020 14:57
@Aljullu
Copy link
Contributor Author

Aljullu commented Apr 24, 2020

I rebased this branch to fix the conflicts that have recently appeared.

@ItsJonQ
Copy link

ItsJonQ commented Apr 28, 2020

Thanks for working on this @Aljullu!

I just gave it a shot with voiceover.
I think the voiceover announcing the current item is helpful.
However, I think (I may be wrong) that the group label needs to be announced somehow.

For example, the fontsize picker, the voiceover should mention "font size" group, "normal" currently selected.

I believe aria tags could help with this?

cc'ing @diegohaz for a11y guidance 😊

@diegohaz
Copy link
Member

Ideally, it should be done by adding both the button and the label IDs to the aria-labelledby prop:

<span id="label">Label</span>
<button id="button" aria-labelledby="label button">
  Content
</button>

It should be announced as Label Content. Or, for the storybook example here: "Font size Large".

This is exactly how the Collapsible Dropdown Listbox Example is built on the WAI-ARIA recommendations site.

See the link above

It seems that Downshift does this by default, but we're overwriting it here:

// This is needed because some speech recognition software don't support `aria-labelledby`.
'aria-label': label,
'aria-labelledby': undefined,

Which led me to #17926 (comment):

Any reason not to make this an actual instead of using aria-labelledby? Then we might not need the aria-label. Having both, VoiceOver and NVDA are announcing the button as "Font size font size". Would be good to avoid the repetition.

Dragon Speech still needs the explicit aria label on the toggle to be able to click on it.

I would say that, if Dragon Speech doesn't support it, it's something that should be fixed on their side. We should follow the WAI-ARIA recommendations to avoid future issues. So, my suggestion here is to stick with Downshift defaults.

cc'ing @epiqueras and @enriquesanchez as they may have other opinions on this. 😊

@enriquesanchez
Copy link
Contributor

Being one of the biggest speech recognition apps, it was decided to add the necessary support for it. It was a tough call knowing that Dragon Speech is widely used by the a11y community but the company behind it is not really invested in accessibility.

@diegohaz
Copy link
Member

diegohaz commented May 1, 2020

@enriquesanchez Are you able to use Dragon to interact with the dropdown on this page: https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html ?

If so, there might be some solution there that we could use. Otherwise, I think we should at least file a bug on Dragon or something.

@enriquesanchez
Copy link
Contributor

@diegohaz Just tried it and I can't interact with the dropdown. It seems that Dragon does not recognize the element.

Base automatically changed from master to trunk March 1, 2021 15:43
@alexstine
Copy link
Contributor

I believe I fixed this here: #33941

I know I tried to fix all the controls in the editor currently. Probably still some that need fixing.

@Aljullu
Copy link
Contributor Author

Aljullu commented Feb 3, 2022

@alexstine you're right, thanks! I just tested it in trunk with Firefox + Orca and the currently selected value is announced to the user when the select is focused.

Closing this PR since it's no longer needed.

@Aljullu Aljullu closed this Feb 3, 2022
@youknowriad youknowriad deleted the fix/21459-customselectcontrol-read-value-on-focus branch February 3, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CustomSelectControl: currently selected value is not presented to screen readers
6 participants