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

Please don't add help text to live region when combobox drops down #1244

Closed
8 tasks
carmacleod opened this issue Mar 1, 2021 · 10 comments · Fixed by #1579 or #1582
Closed
8 tasks

Please don't add help text to live region when combobox drops down #1244

carmacleod opened this issue Mar 1, 2021 · 10 comments · Fixed by #1579 or #1582
Labels
enhancement wip Work in progress

Comments

@carmacleod
Copy link

carmacleod commented Mar 1, 2021

Remove the a11y status message functionality from the hooks

  • remove getA11yStatusMessage from useSelect.
  • remove getA11ySelectionMessage from useSelect.
  • remove getA11yStatusMessage from useCombobox.
  • remove getA11ySelectionMessage from useCombobox.
  • remove getA11yRemovalMessage from useMultipleSelection.
  • check that status message still works in Downshift.
  • update documentation to instruct developers to use onStateChange and any aria-live technique if they wish to maintain the previous functionality.
  • merge to v9 branch with migration guide updated.

Original Message:

Please do not add the following "help/hint text" to the a11y-status-message live region when a combobox is dropped down:

"6 results are available, use up and down arrow keys to navigate. Press Enter key to select."

Screen reader users can usually turn off the help/hint text for their screen reader (as soon as they become proficient with it) in order to reduce verbosity, and since they wouldn't be able to turn this off, it would get annoying pretty quickly. :)

Regarding "6 results are available":

  • Browsers are required to calculate aria-setsize and aria-posinset for things like options in a listbox. This allows AT to say things like "Beans, 3 of 6", "Rice, 4 of 6". So this info is already available to the screen reader user.

Regarding "use up and down arrow keys to navigate. Press Enter key to select.":

  • role="combobox" communicates this fully without adding any words, i.e. as soon as the user hears "combobox", they already know to use up/down arrow and enter (plus some other keys as well, like home/end).

Please also see this comment, which contains screen reader output showing that this particular live region content is verbose and unnecessary.

@silviuaavram
Copy link
Collaborator

This is great feedback @carmacleod! I believe this feature is based on a JQuery combobox implementation which was probably added before this was supported. We should move forward and remove the status text.

I see that you are still using Downshift. What's preventing your from moving to useCombobox? Issues or lack of support for some features?

@joshblack
Copy link

Hey @silviuaavram! 👋 I can definitely answer from the Carbon side of things.

For context, Carbon is a design system built by IBM and we are currently on our v10.x release (10.0.0 was released ~2 years ago). One of our constraints for this release was that we needed to maintain the current major version for a couple of years. Thankfully we'll be able to move forward to v11 this year 🥳

As a result of this constraint, when updating downshift to newer major versions we had to be careful about doing it in a backward-compatible way. Thankfully most of our components that leverage Downshift have shifted over to the hooks variant with combobox being the last one to move over.

Also, just wanted to say thanks to all the contributors and maintainers of this library and ecosystem. It really helped us out over the years and we all appreciate the hard work that went into this project by everyone involved ❤️

@silviuaavram
Copy link
Collaborator

Thank you for the feedback, I'm really happy that it is helping you.

I will keep this task to track the removal of the a11y status message, for the hooks. I prefer to leave Downshift as it is in terms of functionality, and move forward with the hooks.

Will update on the status of this work. Thanks again!

@domantasjurkus
Copy link
Contributor

Just wanted to +1 the idea of removing the live region - I get announcements No results are available before a network query has finished fetching the results for the combobox.

Looking for a way to work around this at the moment with a custom environment - would be nice to have the a11y status message removed altogether.

@silviuaavram
Copy link
Collaborator

silviuaavram commented Aug 5, 2021 via email

@domantasjurkus
Copy link
Contributor

domantasjurkus commented Aug 5, 2021

getA11yStatusMessage fits my needs perfectly, thank you - oddly, the live docs page do not describe the prop, although I can see it in the readmes for the hooks 🤔 maybe some documentation got stale? (apologies for side-tracking the original issue)

@TrevorRice
Copy link

Is there any way to remove the <div id="a11y-status-message" ...> element entirely? We're using useCombobox but also don't have a need for this live region. I'm currently returning an empty string from getA11yStatusMessage, but the element is still rendered.

@silviuaavram
Copy link
Collaborator

silviuaavram commented Jul 26, 2023

We will remove this functionality from all hooks, since it's not considered necessary anymore. Since it's going to be breaking, it will land in v9. WIP for now.

We will remove the props (all getA11y callbacks) and the status div generated by the hook.

If developers would still need a status aria message announcement, they can add their own using the onStateChange callback and any aria live technique. We will also mention this in the documentation, as well as in the next migration guide.

Thanks eveyone!

@pwolfert
Copy link

For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically how updateA11yStatus is debounced. In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but the WCAG docs do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom getA11ySelectionMessage function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired ${itemToString(selectedItem)} has been selected. gets printed to the a11y-message div. However, due to the debouncing, none of the other getA11ySelectionMessage functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those getA11yMessage functions instead of passing them to updateA11yStatus and then ignored them if they returned, say, undefined, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.

pwolfert added a commit to CMSgov/design-system that referenced this issue Jul 28, 2023
Unfortunately it does not work because of the debouncing done inside Downshift. [Here's my explanation](downshift-js/downshift#1244):

> For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically [how updateA11yStatus is debounced](https://github.com/downshift-js/downshift/blob/master/src/hooks/utils.js#L85). In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but [the WCAG docs](https://www.w3.org/WAI/WCAG21/Understanding/on-input) do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom `getA11ySelectionMessage` function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired `${itemToString(selectedItem)} has been selected.` gets printed to the a11y-message div. However, due to the debouncing, none of the other `getA11ySelectionMessage` functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those `getA11yMessage` functions instead of passing them to `updateA11yStatus` and then ignored them if they returned, say, `undefined`, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.
pwolfert added a commit to CMSgov/design-system that referenced this issue Jul 28, 2023
Unfortunately it does not work because of the debouncing done inside Downshift. [Here's my explanation](downshift-js/downshift#1244):

> For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically [how updateA11yStatus is debounced](https://github.com/downshift-js/downshift/blob/master/src/hooks/utils.js#L85). In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but [the WCAG docs](https://www.w3.org/WAI/WCAG21/Understanding/on-input) do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom `getA11ySelectionMessage` function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired `${itemToString(selectedItem)} has been selected.` gets printed to the a11y-message div. However, due to the debouncing, none of the other `getA11ySelectionMessage` functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those `getA11yMessage` functions instead of passing them to `updateA11yStatus` and then ignored them if they returned, say, `undefined`, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.
pwolfert added a commit to CMSgov/design-system that referenced this issue Jul 31, 2023
…2604)

* Try to fix lost selection message with custom getA11ySelectionMessage fn

Unfortunately it does not work because of the debouncing done inside Downshift. [Here's my explanation](downshift-js/downshift#1244):

> For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically [how updateA11yStatus is debounced](https://github.com/downshift-js/downshift/blob/master/src/hooks/utils.js#L85). In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but [the WCAG docs](https://www.w3.org/WAI/WCAG21/Understanding/on-input) do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom `getA11ySelectionMessage` function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired `${itemToString(selectedItem)} has been selected.` gets printed to the a11y-message div. However, due to the debouncing, none of the other `getA11ySelectionMessage` functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those `getA11yMessage` functions instead of passing them to `updateA11yStatus` and then ignored them if they returned, say, `undefined`, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.

* Get rid of new function that didn't work

See previous commit

* Actually, give full control to user-defined functions

If our users are defining a `getA11yStatusMessage` function, it's because they want to control it.

* [DO-NOT-MERGE] Test code in story

* Revert "[DO-NOT-MERGE] Test code in story"

This reverts commit 405fc07.
pwolfert added a commit to CMSgov/design-system that referenced this issue Jul 31, 2023
…2604)

* Try to fix lost selection message with custom getA11ySelectionMessage fn

Unfortunately it does not work because of the debouncing done inside Downshift. [Here's my explanation](downshift-js/downshift#1244):

> For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically [how updateA11yStatus is debounced](https://github.com/downshift-js/downshift/blob/master/src/hooks/utils.js#L85). In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but [the WCAG docs](https://www.w3.org/WAI/WCAG21/Understanding/on-input) do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom `getA11ySelectionMessage` function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired `${itemToString(selectedItem)} has been selected.` gets printed to the a11y-message div. However, due to the debouncing, none of the other `getA11ySelectionMessage` functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those `getA11yMessage` functions instead of passing them to `updateA11yStatus` and then ignored them if they returned, say, `undefined`, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.

* Get rid of new function that didn't work

See previous commit

* Actually, give full control to user-defined functions

If our users are defining a `getA11yStatusMessage` function, it's because they want to control it.

* [DO-NOT-MERGE] Test code in story

* Revert "[DO-NOT-MERGE] Test code in story"

This reverts commit 405fc07.
pwolfert added a commit to CMSgov/design-system that referenced this issue Aug 8, 2023
…switching (#2599)

* We never did actually export `NativeDialog` even though it looked like it

Remove the export line from our index so it's not confusing. The export never worked. We haven't released this component to the public.

* Basic, untested FilterDialog commponent added to docs package

* WIP: Started with a dumb component and logical component, but...

The problem with that is that the versions list is dynamic; it's based on the currently selected theme. That means I need logic inside the component that renders the dialog to dynamically change the options in the version dropdown based on the theme

* Moved it all into one component, and it makes sense

* Hook it up and start working out kinks in version/theme switcher

Problem right now is that if you previously had one theme selected and you select a different one, it will think you changed versions too because the version numbers don't match. While I could convert all versions to the core version equivalent to compare, what if it can't find the corresponding core version and has to fall back? Then the comparison would be wrong. But that doesn't happen, because there are no versions that don't have core equivalents. So maybe that'll actually work

* Fixed `useTheme` causing a second render for every component that uses it

It would always start with "core" and then switch to the actual current value. That second render was causing problems in the theme switcher

* Reorganize modules and create a basic ThemeVersionSection for the sidenav

* Use inverse colors

* Fix bug in my useTheme fix

* Some work on styling

* Styling for the triggering button

* Add arrow icon to the "Change settings" button

* Use theme `displayName` in sidebar

* Remove old theme and version switcher components

* Turns out we don't need the onExit function

* Add some basic unit tests

* Try to fix lost selection message with custom getA11ySelectionMessage fn

Unfortunately it does not work because of the debouncing done inside Downshift. [Here's my explanation](downshift-js/downshift#1244):

> For what it's worth, I have just come across an issue with the existing a11y-message implementation and specifically [how updateA11yStatus is debounced](https://github.com/downshift-js/downshift/blob/master/src/hooks/utils.js#L85). In my application, I have one dropdown on a page that modifies the available options for another dropdown further down the page. (This isn't ideal from an accessibility standpoint, but [the WCAG docs](https://www.w3.org/WAI/WCAG21/Understanding/on-input) do say we can do that if we warn users that it's going to happen, and it will take a while before we can design out all the instances of it across multiple applications.) I wanted to write a custom `getA11ySelectionMessage` function that would determine when it's one of those incidentally changed dropdowns rather than the one the user is interacting with so that only the desired `${itemToString(selectedItem)} has been selected.` gets printed to the a11y-message div. However, due to the debouncing, none of the other `getA11ySelectionMessage` functions even get called except for the last one, which is the one I don't want to announce. If Downshift called those `getA11yMessage` functions instead of passing them to `updateA11yStatus` and then ignored them if they returned, say, `undefined`, this plan would work. But right now the agency from each individual dropdown is taken from it to determine whether its status message should be announced. It's getting lost in the debounce.

* Get rid of new function that didn't work

See previous commit

* Pivot to using two dialogs

This should be a much more straightforward experience and not create accessibility problems with the theme dropdown dynamically changing the options of the version dropdown.

* Move focus back to the triggering button when closing a filter dialog

Need to figure out a clean way of making this the easy thing to do for teams

* Add a close button

* Add the border radius used for dropdowns

* Allow the close icon label to be announced

Forgot that it is off by default

* Show which design system theme we're on in the page (tab) title

* Fix HealthCare.gov theme text wrapping

Don't constrain the theme/version section more than we need to

* Update snapshot

* Better screen reader text for the theme and version buttons

* Move the theme and version section out of side nav on mobile

* Fine tune the spacing after making the hit box large enough

* Fix mobile spacing on page header

* Fixes according to feedback

* Don't let multiple dialogs be open at once

* Update this prop's documentation to make it more clear what they need to do

* Update snapshot
@silviuaavram
Copy link
Collaborator

We're on track with this change. It should be done in a few days, and will involve releasing an alpha version, then if all is good, a major version (v9 most probably).

We keep the getStatusMessage prop, for all hooks, which will be called with the state values, but without any default implementation. If user does not pass any function for building an a11y message, nothing will happen. If they do, then we will show it in the status aria-live div, then clean it up, then remove the div on unmount.

This is a middle ground between removing it completely and keeping it this way. The docs will be updated as well and also a migration guide. If there are more tips on this, please go ahead. Thank you!

@silviuaavram silviuaavram mentioned this issue Mar 15, 2024
Merged
5 tasks
silviuaavram added a commit that referenced this issue Mar 20, 2024
<!--
Thanks for your interest in the project. Bugs filed and PRs submitted are appreciated!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

If you're new to contributing to open source projects, you might find this free
video course helpful: http://kcd.im/pull-request

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->

**What**:
BREAKING CHANGE: Release Downshift v9.

<!-- Why are these changes necessary? -->

**Why**:
Release the changes in:
- #1580
- #1579
- #1583

Closes #1322.
Closes #1244.
Closes #1227
Closes #1225.
<!-- How were these changes implemented? -->

**How**:
Merged the PRs in this branch.
<!-- Have you done all of these things?  -->

**Checklist**:

<!-- add "N/A" to the end of each line that's irrelevant to your changes -->

<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->

- [x] Documentation
- [x] Tests
- [x] TypeScript Types
- [ ] Flow Types
- [x] Ready to be merged
      <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->

<!-- feel free to add additional comments -->
mergify bot pushed a commit to SvenKirschbaum/musikbot-frontend that referenced this issue Mar 22, 2024
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [downshift](https://downshift-js.com) ([source](https://togithub.com/downshift-js/downshift)) | [`^8.3.1` -> `^9.0.0`](https://renovatebot.com/diffs/npm/downshift/8.5.0/9.0.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/downshift/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/downshift/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/downshift/8.5.0/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/downshift/8.5.0/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>downshift-js/downshift (downshift)</summary>

### [`v9.0.0`](https://togithub.com/downshift-js/downshift/releases/tag/v9.0.0)

[Compare Source](https://togithub.com/downshift-js/downshift/compare/v8.5.0...v9.0.0)

##### Features

-   V9 ([#&#8203;1582](https://togithub.com/downshift-js/downshift/issues/1582)) ([5b0d503](https://togithub.com/downshift-js/downshift/commit/5b0d5031c1c11455ba18fa1516a259b4ed9357a1))

##### BREAKING CHANGES

-   Release Downshift v9.



**Why**:
Release the changes in:

-   [downshift-js/downshift#1580
-   [downshift-js/downshift#1579
-   [downshift-js/downshift#1583

Closes [downshift-js/downshift#1322.
Closes [downshift-js/downshift#1244.
Closes [downshift-js/downshift#1227
Closes [downshift-js/downshift#1225.



**How**:
Merged the PRs in this branch.



**Checklist**:





-   \[x] Documentation
-   \[x] Tests
-   \[x] TypeScript Types
-   \[ ] Flow Types
-   \[x] Ready to be merged 



</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/SvenKirschbaum/musikbot-frontend).
guilhermespopolin pushed a commit to HedvigInsurance/racoon that referenced this issue Mar 27, 2024
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [downshift](https://downshift-js.com) ([source](https://togithub.com/downshift-js/downshift)) | [`8.5.0` -> `9.0.0`](https://renovatebot.com/diffs/npm/downshift/8.5.0/9.0.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/downshift/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/downshift/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/downshift/8.5.0/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/downshift/8.5.0/9.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>downshift-js/downshift (downshift)</summary>

### [`v9.0.0`](https://togithub.com/downshift-js/downshift/releases/tag/v9.0.0)

[Compare Source](https://togithub.com/downshift-js/downshift/compare/v8.5.0...v9.0.0)

##### Features

-   V9 ([#&#8203;1582](https://togithub.com/downshift-js/downshift/issues/1582)) ([5b0d503](https://togithub.com/downshift-js/downshift/commit/5b0d5031c1c11455ba18fa1516a259b4ed9357a1))

##### BREAKING CHANGES

-   Release Downshift v9.

<!-- Why are these changes necessary? -->

**Why**:
Release the changes in:

-   [downshift-js/downshift#1580
-   [downshift-js/downshift#1579
-   [downshift-js/downshift#1583

Closes [downshift-js/downshift#1322.
Closes [downshift-js/downshift#1244.
Closes [downshift-js/downshift#1227
Closes [downshift-js/downshift#1225.

<!-- How were these changes implemented? -->

**How**:
Merged the PRs in this branch.

<!-- Have you done all of these things?  -->

**Checklist**:

<!-- add "N/A" to the end of each line that's irrelevant to your changes -->

<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->

-   \[x] Documentation
-   \[x] Tests
-   \[x] TypeScript Types
-   \[ ] Flow Types
-   \[x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->

<!-- feel free to add additional comments -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 2pm every weekday" in timezone Europe/Stockholm, Automerge - "after 6am and before 2pm every weekday" in timezone Europe/Stockholm.

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/HedvigInsurance/racoon).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement wip Work in progress
Projects
None yet
6 participants