feat(a11y): allow to queue announcements in LiveAnnouncer #17986
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
googlebot
added
the
cla: no
|
@googlebot I fixed it. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
googlebot
added
cla: yes
| return this._ngZone.runOutsideAngular(() => { | ||
| return new Promise(resolve => { | ||
| clearTimeout(this._previousTimeout); | ||
| this._previousTimeout = setTimeout(() => { |
There was a problem hiding this comment.
Should run outside the NgZone.
There was a problem hiding this comment.
I'm unfamiliar with how zone.js works. The promise is already wrapped in ngZone.runOutsideAngular, is it enough or there should be another similar wrapper around setTimeout?
Also this particular code was just moved from another location. If it's a bug, let me know, it's probably better to fix it in a separate PR
Standard LiveAnnouncer operates with a single aria-live element, which means that if several announcements happen at the same time, only the latest one is actually announced. This change adds `useQueue` configuration to LiveAnnouncerDefaultOptions. When it's set to `true` LiveAnnouncer creates a separate aria-live element for each of the announcements and places all messages in the queue, so they are announced one by one. Fixes #17852
|
|
||
| /** | ||
| * Indicates whether announcements should be queued or each next one should | ||
| * override the previous one. |
There was a problem hiding this comment.
| * override the previous one. | |
| * override the previous one. Defaults to `false`. |
| const ARIA_LIVE_ANNOUNCER_REMOVAL_DELAY_MS = 1000; | ||
|
|
||
| /** Class for the hidden div element with an aria-live attribute. */ | ||
| const LIVE_ELEMENT_CLASS = 'cdk-live-announcer-element'; |
There was a problem hiding this comment.
| const LIVE_ELEMENT_CLASS = 'cdk-live-announcer-element'; | |
| const LIVE_ELEMENT_CSS_CLASS = 'cdk-live-announcer-element'; |
| message: string, politeness: AriaLivePoliteness, duration?: number): Promise<void> { | ||
| const liveElement = this._createLiveElement(); | ||
| // TODO: ensure changing the politeness works on all environments we support. | ||
| liveElement.setAttribute('aria-live', politeness); |
There was a problem hiding this comment.
The politeness should be set at the same time the queued announcement is made, otherwise it will get out of sync. Both the immediate and queued code path should really call the same method that sets the politeness and updates the textContent
| * queued live announce messages are read, even if the node containing them is | ||
| * removed before the message starts playing. | ||
| */ | ||
| const ARIA_LIVE_ANNOUNCER_REMOVAL_DELAY_MS = 1000; |
There was a problem hiding this comment.
Will screen-readers continue reading a long message if it's removed from the DOM before the announcement is done? I worry that some will and that one second is too short.
| liveElement.setAttribute('aria-live', politeness); | ||
|
|
||
| return new Promise(resolve => { | ||
| this._taskQueue.enqueue(() => { |
There was a problem hiding this comment.
Upon looking at the code, I think the TaskQueue isn't strictly necessary and that this could be done a bit more simply. You could instead keep two arrays, _assertiveQueue and _politeQueue. After any given announcement finishes, pop the front message from _assertiveQueue if not empty, then _politeQueue, and repeat.
andrewseguin
removed
the
cla: yes
|
Closing due to inactivity since the latest comments. Feel free to re-open a new PR if you'd like to try and get this landed |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Standard LiveAnnouncer operates with a single aria-live element, which means that if several
announcements happen at the same time, only the latest one is actually announced. This change adds
useQueueconfiguration to LiveAnnouncerDefaultOptions. When it's set totrueLiveAnnouncercreates a separate aria-live element for each of the announcements and places all messages in
the queue, so they are announced one by one.
Fixes #17852