-
Notifications
You must be signed in to change notification settings - Fork 203
fix: Ensure rapid successive live announcements are not lost #3987
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3987 +/- ##
=======================================
Coverage 97.24% 97.24%
=======================================
Files 858 858
Lines 25346 25358 +12
Branches 9014 9016 +2
=======================================
+ Hits 24647 24659 +12
Misses 652 652
Partials 47 47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a9c7947 to
e16a83c
Compare
| * Screen readers throttle announcements to avoid overwhelming users. | ||
| * A 500ms gap ensures each announcement is detected and spoken. | ||
| */ | ||
| private static readonly MIN_ANNOUNCEMENT_GAP_MS = 500; |
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.
500 seems quite long, maybe you can get away with 250?
| if (this._timeoutId === undefined) { | ||
| this._timeoutId = setTimeout(() => this._updateElement(false), this.delay * 1000); | ||
| // Clear any existing timeout to ensure the latest announcement is used. | ||
| if (this._timeoutId !== undefined) { |
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.
This significantly changes the behavior of the live region. Previously, we throttled announcements — if the live region triggered 50 times in 2 seconds, it would only trigger an update twice (the first one, and another one at the end of the 2s period).
After this update, updates are debounced, not throttled (since each call clears the timeout) and (if I can visualize how this works correctly) if this gets called multiple times, each call will just add to the _nextAvailableSlot. So if trigger the live region 50 times in 2 seconds, you'll only hear one announcement 25 seconds later.
The weird paradox here is that we do want to deduplicate and remove announcements if they happen too frequently (which is why this controller exists), but we don't want to do it between different live regions. Maybe you can preserve the existing behavior, give each live region a unique incrementing ID, and only do the slot behavior if the last slot didn't belong to the current live region. Or, do this check inside _updateElement so that existing behavior doesn't need to be messed with unless there's the edge case that another live region fired and we have to delay our update a little bit more?
Description
In some cases, when two live regions are used and updated nearly at the same time, one of the announcements sometimes get missed by screen readers (tested with VO on Mac).
With this change, multiple LiveRegion instances are automatically coordinated to prevent
Related links, issue #, if available:
AWSUI-61345How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.