Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Conversation

0ddfell0w
Copy link
Contributor

@0ddfell0w 0ddfell0w commented Jul 19, 2017

Edit:
change how dialogs hide background elements to avoid hiding live regions, ensuring that an autocomplete inside a dialog will not have its live region aria-hidden

Fixes #10804


Out of date:
add moveLiveRegion fn that moves an autocomplete's live region off document.body
and makes it a child of md-autocomplete. this prevents dialogs from
mistakenly hiding an autocomplete's live region

Fixes #10804

@googlebot googlebot added cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ labels Jul 19, 2017
@0ddfell0w
Copy link
Contributor Author

@jelbourn

*/
function moveLiveRegion () {
angular.element($mdLiveAnnouncer._liveElement).detach();
elements.$.main.append($mdLiveAnnouncer._liveElement)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $mdLiveAnnouncer is a global singleton; it does not belong to the autocomplete. With this, each autocomplete will "steal" the aria-live element when it's created. Said element will also be destroyed if whichever autocomplete stole it most recently is destroyed, breaking all subsequent announcing.

The easiest way to fix this issue is probably to have the dialog specifically ignore the aria-live element when marking everything as aria-hidden.

@jelbourn jelbourn added the g3: pr This PR was posted by an internal or external product team. label Jul 20, 2017
@0ddfell0w 0ddfell0w force-pushed the autocomplete_live_region_inside_dialog branch from 7b78cc3 to 19f90bd Compare July 20, 2017 18:19
@0ddfell0w 0ddfell0w changed the title fix(autocomplete): move autocomplete live region inside md-autocomplete fix(dialog):prevent dialogs from hiding live regions Jul 20, 2017
@0ddfell0w
Copy link
Contributor Author

@jelbourn thanks for explaining, didn't know services were singletons. took your suggestion, which works as well. 🙏 👀

@0ddfell0w 0ddfell0w force-pushed the autocomplete_live_region_inside_dialog branch from 19f90bd to e0e638c Compare July 20, 2017 21:20
if (element !== children[i] && !isNodeOneOf(children[i], ['SCRIPT', 'STYLE'])) {
if (element !== children[i] &&
!isNodeOneOf(children[i], ['SCRIPT', 'STYLE']) &&
!children[i].hasAttribute('aria-live')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a unit test for this change?

Here's an existing test for this behavior:

it('should apply aria-hidden to siblings', inject(function($mdDialog, $rootScope, $timeout) {

You should be able to follow that to create a new test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

change how dialogs hide background elements to avoid hiding live
regions, ensuring that an autocomplete inside a dialog will not have
its live region aria-hidden. Adds unit test

Fixes angular#10804
@0ddfell0w 0ddfell0w force-pushed the autocomplete_live_region_inside_dialog branch from e0e638c to 7c14825 Compare July 21, 2017 17:35
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added pr: merge ready This PR is ready for a caretaker to review and removed needs: presubmit labels Jul 21, 2017
@kara kara merged commit b084f3d into angular:master Jul 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ g3: pr This PR was posted by an internal or external product team. pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants