Skip to content

Commit abffbf3

Browse files
committed
fix(material/dialog): improve screen reader support when opened
- notify screen reader users that they have entered a dialog - previously only the focused element would be read i.e. "Close Button Press Search plus Space to activate" - now the screen reader user gets the normal dialog behavior, which is to read the dialog title, role, content, and then tell the user about the focused element - this matches the guidance here: https://www.w3.org/TR/wai-aria-practices-1.1/examples/dialog-modal/dialog.html - add flushes to mdc-dialog tests to allow dialogs to animate open Fixes #21840
1 parent 91fa44e commit abffbf3

File tree

5 files changed

+75
-22
lines changed

5 files changed

+75
-22
lines changed

src/material-experimental/mdc-dialog/dialog.spec.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ describe('MDC-based MatDialog', () => {
383383

384384
it('should notify the observers if all open dialogs have finished closing', fakeAsync(() => {
385385
const ref1 = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
386+
flush();
386387
const ref2 = dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef});
387388
const spy = jasmine.createSpy('afterAllClosed spy');
388389

@@ -615,7 +616,9 @@ describe('MDC-based MatDialog', () => {
615616

616617
it('should close all of the dialogs', fakeAsync(() => {
617618
dialog.open(PizzaMsg);
619+
flush();
618620
dialog.open(PizzaMsg);
621+
flush();
619622
dialog.open(PizzaMsg);
620623

621624
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
@@ -629,6 +632,7 @@ describe('MDC-based MatDialog', () => {
629632

630633
it('should close all dialogs when the user goes forwards/backwards in history', fakeAsync(() => {
631634
dialog.open(PizzaMsg);
635+
flush();
632636
dialog.open(PizzaMsg);
633637

634638
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
@@ -642,6 +646,7 @@ describe('MDC-based MatDialog', () => {
642646

643647
it('should close all open dialogs when the location hash changes', fakeAsync(() => {
644648
dialog.open(PizzaMsg);
649+
flush();
645650
dialog.open(PizzaMsg);
646651

647652
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
@@ -655,7 +660,9 @@ describe('MDC-based MatDialog', () => {
655660

656661
it('should close all of the dialogs when the injectable is destroyed', fakeAsync(() => {
657662
dialog.open(PizzaMsg);
663+
flush();
658664
dialog.open(PizzaMsg);
665+
flush();
659666
dialog.open(PizzaMsg);
660667

661668
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
@@ -685,6 +692,7 @@ describe('MDC-based MatDialog', () => {
685692

686693
it('should allow the consumer to disable closing a dialog on navigation', fakeAsync(() => {
687694
dialog.open(PizzaMsg);
695+
flush();
688696
dialog.open(PizzaMsg, {closeOnNavigation: false});
689697

690698
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
@@ -772,14 +780,15 @@ describe('MDC-based MatDialog', () => {
772780
expect(dialogRef.componentInstance).toBeFalsy('Expected reference to have been cleared.');
773781
}));
774782

775-
it('should assign a unique id to each dialog', () => {
783+
it('should assign a unique id to each dialog', fakeAsync(() => {
776784
const one = dialog.open(PizzaMsg);
785+
flush();
777786
const two = dialog.open(PizzaMsg);
778787

779788
expect(one.id).toBeTruthy();
780789
expect(two.id).toBeTruthy();
781790
expect(one.id).not.toBe(two.id);
782-
});
791+
}));
783792

784793
it('should allow for the id to be overwritten', () => {
785794
const dialogRef = dialog.open(PizzaMsg, {id: 'pizza'});
@@ -1064,7 +1073,7 @@ describe('MDC-based MatDialog', () => {
10641073
expect(document.activeElement!.id)
10651074
.not.toBe(
10661075
'dialog-trigger',
1067-
'Expcted the focus not to have changed before the animation finishes.');
1076+
'Expected the focus not to have changed before the animation finishes.');
10681077

10691078
flushMicrotasks();
10701079
viewContainerFixture.detectChanges();
@@ -1789,6 +1798,15 @@ describe('MDC-based MatDialog with animations enabled', () => {
17891798
flush();
17901799
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
17911800
}));
1801+
1802+
it('should not allow another dialog to open until the previous dialog finished animating open',
1803+
() => {
1804+
const openTwoDialogs = () => {
1805+
dialog.open(PizzaMsg);
1806+
dialog.open(PizzaMsg);
1807+
};
1808+
expect(openTwoDialogs).toThrowError(/Ignoring attempt to open dialog/);
1809+
});
17921810
});
17931811

17941812
@Directive({selector: 'dir-with-view-container'})

src/material-experimental/mdc-dialog/dialog.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {Inject, Injectable, InjectionToken, Injector, Optional, SkipSelf} from '
1212
import {_MatDialogBase, MatDialogConfig} from '@angular/material/dialog';
1313
import {MatDialogContainer} from './dialog-container';
1414
import {MatDialogRef} from './dialog-ref';
15+
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
1516

1617
/** Injection token that can be used to access the data that was passed in to a dialog. */
1718
export const MAT_DIALOG_DATA = new InjectionToken<any>('MatMdcDialogData');
@@ -52,9 +53,11 @@ export class MatDialog extends _MatDialogBase<MatDialogContainer> {
5253
@Optional() location: Location,
5354
@Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig,
5455
@Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
55-
@Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer) {
56+
@Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer,
57+
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: 'NoopAnimations'|
58+
'BrowserAnimations') {
5659
super(
5760
overlay, injector, defaultOptions, parentDialog, overlayContainer, scrollStrategy,
58-
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA);
61+
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA, animationMode);
5962
}
6063
}

src/material/dialog/dialog-container.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
105105
// Save the previously focused element. This element will be re-focused
106106
// when the dialog closes.
107107
this._capturePreviouslyFocusedElement();
108-
// Move focus onto the dialog immediately in order to prevent the user
109-
// from accidentally opening multiple dialogs at the same time.
110-
this._focusDialogContainer();
111108
}
112109

113110
/**
@@ -154,7 +151,7 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
154151
const focusContainer = !this._config.autoFocus || !this._focusTrap.focusInitialElement();
155152

156153
if (focusContainer) {
157-
this._elementRef.nativeElement.focus();
154+
this._focusDialogContainer();
158155
}
159156
}
160157
}
@@ -165,14 +162,20 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
165162
// ready in instances where change detection has to run first. To deal with this, we simply
166163
// wait for the microtask queue to be empty.
167164
if (this._config.autoFocus) {
168-
this._focusTrap.focusInitialElementWhenReady();
165+
this._focusTrap.focusInitialElementWhenReady().then(focusedSuccessfully => {
166+
// If we weren't able to find a focusable element in the dialog, then focus the dialog
167+
// container instead.
168+
if (!focusedSuccessfully) {
169+
this._focusDialogContainer();
170+
}
171+
});
169172
} else if (!this._containsFocus()) {
170173
// Otherwise ensure that focus is on the dialog container. It's possible that a different
171174
// component tried to move focus while the open animation was running. See:
172175
// https://github.com/angular/components/issues/16215. Note that we only want to do this
173176
// if the focus isn't inside the dialog already, because it's possible that the consumer
174177
// turned off `autoFocus` in order to move focus themselves.
175-
this._elementRef.nativeElement.focus();
178+
this._focusDialogContainer();
176179
}
177180
}
178181

src/material/dialog/dialog.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,7 @@ describe('MatDialog', () => {
12271227

12281228
tick(500);
12291229
viewContainerFixture.detectChanges();
1230-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1230+
expect(lastFocusOrigin!).toBe('program');
12311231

12321232
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
12331233

@@ -1260,7 +1260,7 @@ describe('MatDialog', () => {
12601260

12611261
tick(500);
12621262
viewContainerFixture.detectChanges();
1263-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1263+
expect(lastFocusOrigin!).toBe('program');
12641264

12651265
const backdrop = overlayContainerElement
12661266
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
@@ -1296,7 +1296,7 @@ describe('MatDialog', () => {
12961296

12971297
tick(500);
12981298
viewContainerFixture.detectChanges();
1299-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1299+
expect(lastFocusOrigin!).toBe('program');
13001300

13011301
const closeButton = overlayContainerElement
13021302
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1333,7 +1333,7 @@ describe('MatDialog', () => {
13331333

13341334
tick(500);
13351335
viewContainerFixture.detectChanges();
1336-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1336+
expect(lastFocusOrigin!).toBe('program');
13371337

13381338
const closeButton = overlayContainerElement
13391339
.querySelector('button[mat-dialog-close]') as HTMLElement;

src/material/dialog/dialog.ts

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ import {
3030
TemplateRef,
3131
Type,
3232
} from '@angular/core';
33-
import {defer, Observable, of as observableOf, Subject} from 'rxjs';
33+
import {defer, Observable, of as observableOf, Subject, Subscription} from 'rxjs';
3434
import {startWith} from 'rxjs/operators';
3535
import {MatDialogConfig} from './dialog-config';
3636
import {MatDialogContainer, _MatDialogContainerBase} from './dialog-container';
3737
import {MatDialogRef} from './dialog-ref';
38+
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
3839

3940

4041
/** Injection token that can be used to access the data that was passed in to a dialog. */
@@ -77,6 +78,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
7778
private readonly _afterOpenedAtThisLevel = new Subject<MatDialogRef<any>>();
7879
private _ariaHiddenElements = new Map<Element, string|null>();
7980
private _scrollStrategy: () => ScrollStrategy;
81+
private _dialogAnimatingOpen = false;
82+
private _animationStateSubscriptions: Subscription;
8083

8184
/** Keeps track of the currently-open dialogs. */
8285
get openDialogs(): MatDialogRef<any>[] {
@@ -111,7 +114,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
111114
scrollStrategy: any,
112115
private _dialogRefConstructor: Type<MatDialogRef<any>>,
113116
private _dialogContainerType: Type<C>,
114-
private _dialogDataToken: InjectionToken<any>) {
117+
private _dialogDataToken: InjectionToken<any>,
118+
private _animationMode?: 'NoopAnimations' | 'BrowserAnimations') {
115119
this._scrollStrategy = scrollStrategy;
116120
}
117121

@@ -144,9 +148,30 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
144148
(typeof ngDevMode === 'undefined' || ngDevMode)) {
145149
throw Error(`Dialog with id "${config.id}" exists already. The dialog id must be unique.`);
146150
}
151+
if (this._dialogAnimatingOpen) {
152+
throw Error(`Ignoring attempt to open dialog with id "${
153+
config.id}" since another dialog is currently animating open.`);
154+
}
147155

148156
const overlayRef = this._createOverlay(config);
149157
const dialogContainer = this._attachDialogContainer(overlayRef, config);
158+
if (this._animationMode !== 'NoopAnimations') {
159+
const animationStateSubscription =
160+
dialogContainer._animationStateChanged.subscribe((dialogAnimationEvent) => {
161+
if (dialogAnimationEvent.state === 'opening') {
162+
this._dialogAnimatingOpen = true;
163+
}
164+
if (dialogAnimationEvent.state === 'opened') {
165+
this._dialogAnimatingOpen = false;
166+
animationStateSubscription.unsubscribe();
167+
}
168+
});
169+
if (!this._animationStateSubscriptions) {
170+
this._animationStateSubscriptions = new Subscription();
171+
}
172+
this._animationStateSubscriptions.add(animationStateSubscription);
173+
}
174+
150175
const dialogRef = this._attachDialogContent<T, R>(componentOrTemplateRef,
151176
dialogContainer,
152177
overlayRef,
@@ -188,6 +213,10 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
188213
this._closeDialogs(this._openDialogsAtThisLevel);
189214
this._afterAllClosedAtThisLevel.complete();
190215
this._afterOpenedAtThisLevel.complete();
216+
// Clean up any subscriptions to dialogs that never finished opening.
217+
if (this._animationStateSubscriptions) {
218+
this._animationStateSubscriptions.unsubscribe();
219+
}
191220
}
192221

193222
/**
@@ -392,19 +421,19 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
392421
@Injectable()
393422
export class MatDialog extends _MatDialogBase<MatDialogContainer> {
394423
constructor(
395-
overlay: Overlay,
396-
injector: Injector,
424+
overlay: Overlay, injector: Injector,
397425
/**
398426
* @deprecated `_location` parameter to be removed.
399427
* @breaking-change 10.0.0
400428
*/
401429
@Optional() location: Location,
402430
@Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig,
403431
@Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
404-
@Optional() @SkipSelf() parentDialog: MatDialog,
405-
overlayContainer: OverlayContainer) {
432+
@Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer,
433+
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: 'NoopAnimations'|
434+
'BrowserAnimations') {
406435
super(overlay, injector, defaultOptions, parentDialog, overlayContainer, scrollStrategy,
407-
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA);
436+
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA, animationMode);
408437
}
409438
}
410439

0 commit comments

Comments
 (0)