-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(material/snack-bar): live region not working when modal is open #26725
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,11 +14,13 @@ import { | |
Directive, | ||
ElementRef, | ||
EmbeddedViewRef, | ||
inject, | ||
NgZone, | ||
OnDestroy, | ||
ViewChild, | ||
ViewEncapsulation, | ||
} from '@angular/core'; | ||
import {DOCUMENT} from '@angular/common'; | ||
import {matSnackBarAnimations} from './snack-bar-animations'; | ||
import { | ||
BasePortalOutlet, | ||
|
@@ -34,12 +36,17 @@ import {AnimationEvent} from '@angular/animations'; | |
import {take} from 'rxjs/operators'; | ||
import {MatSnackBarConfig} from './snack-bar-config'; | ||
|
||
let uniqueId = 0; | ||
|
||
/** | ||
* Base class for snack bar containers. | ||
* @docs-private | ||
*/ | ||
@Directive() | ||
export abstract class _MatSnackBarContainerBase extends BasePortalOutlet implements OnDestroy { | ||
private _document = inject(DOCUMENT); | ||
private _trackedModals = new Set<Element>(); | ||
|
||
/** The number of milliseconds to wait before announcing the snack bar's content. */ | ||
private readonly _announceDelay: number = 150; | ||
|
||
|
@@ -73,6 +80,9 @@ export abstract class _MatSnackBarContainerBase extends BasePortalOutlet impleme | |
*/ | ||
_role?: 'status' | 'alert'; | ||
|
||
/** Unique ID of the aria-live element. */ | ||
readonly _liveElementId = `mat-snack-bar-container-live-${uniqueId++}`; | ||
|
||
constructor( | ||
private _ngZone: NgZone, | ||
protected _elementRef: ElementRef<HTMLElement>, | ||
|
@@ -188,6 +198,7 @@ export abstract class _MatSnackBarContainerBase extends BasePortalOutlet impleme | |
/** Makes sure the exit callbacks have been invoked when the element is destroyed. */ | ||
ngOnDestroy() { | ||
this._destroyed = true; | ||
this._clearFromModals(); | ||
this._completeExit(); | ||
} | ||
|
||
|
@@ -220,6 +231,54 @@ export abstract class _MatSnackBarContainerBase extends BasePortalOutlet impleme | |
element.classList.add(panelClasses); | ||
} | ||
} | ||
|
||
this._exposeToModals(); | ||
} | ||
|
||
/** | ||
* Some browsers won't expose the accessibility node of the live element if there is an | ||
* `aria-modal` and the live element is outside of it. This method works around the issue by | ||
* pointing the `aria-owns` of all modals to the live element. | ||
*/ | ||
private _exposeToModals() { | ||
// TODO(crisbeto): consider de-duplicating this with the `LiveAnnouncer`. | ||
// Note that the selector here is limited to CDK overlays at the moment in order to reduce the | ||
// section of the DOM we need to look through. This should cover all the cases we support, but | ||
// the selector can be expanded if it turns out to be too narrow. | ||
const id = this._liveElementId; | ||
const modals = this._document.querySelectorAll( | ||
'body > .cdk-overlay-container [aria-modal="true"]', | ||
); | ||
|
||
for (let i = 0; i < modals.length; i++) { | ||
const modal = modals[i]; | ||
const ariaOwns = modal.getAttribute('aria-owns'); | ||
this._trackedModals.add(modal); | ||
|
||
if (!ariaOwns) { | ||
modal.setAttribute('aria-owns', id); | ||
} else if (ariaOwns.indexOf(id) === -1) { | ||
modal.setAttribute('aria-owns', ariaOwns + ' ' + id); | ||
} | ||
} | ||
} | ||
|
||
/** Clears the references to the live element from any modals it was added to. */ | ||
private _clearFromModals() { | ||
this._trackedModals.forEach(modal => { | ||
const ariaOwns = modal.getAttribute('aria-owns'); | ||
|
||
if (ariaOwns) { | ||
const newValue = ariaOwns.replace(this._liveElementId, '').trim(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if it would be in the middle of other ids? e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that would be fine since it would just leave a whitespace which the browser is going to ignore. |
||
|
||
if (newValue.length > 0) { | ||
modal.setAttribute('aria-owns', newValue); | ||
} else { | ||
modal.removeAttribute('aria-owns'); | ||
} | ||
} | ||
}); | ||
this._trackedModals.clear(); | ||
} | ||
|
||
/** Asserts that no content is already attached to the container. */ | ||
|
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.
Nit: Could use
for (const modal of modals)
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.
That doesn't work, because
querySelectorAll
returns aNodeList
.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.
A node list is iterable, so it should work: https://developer.mozilla.org/en-US/docs/Web/API/NodeList
But no need to follow-up. As said, a nit.