Skip to content

Commit 5dd0503

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 - Avoid opening multiple of the same dialog before animations complete by returning the previous `MatDialogRef` - update material/dialog API golden file Fixes #21840
1 parent 0114ccd commit 5dd0503

File tree

6 files changed

+85
-26
lines changed

6 files changed

+85
-26
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -774,14 +774,14 @@ describe('MDC-based MatDialog', () => {
774774
.withContext('Expected reference to have been cleared.').toBeFalsy();
775775
}));
776776

777-
it('should assign a unique id to each dialog', () => {
777+
it('should assign a unique id to each dialog', fakeAsync(() => {
778778
const one = dialog.open(PizzaMsg);
779779
const two = dialog.open(PizzaMsg);
780780

781781
expect(one.id).toBeTruthy();
782782
expect(two.id).toBeTruthy();
783783
expect(one.id).not.toBe(two.id);
784-
});
784+
}));
785785

786786
it('should allow for the id to be overwritten', () => {
787787
const dialogRef = dialog.open(PizzaMsg, {id: 'pizza'});
@@ -1200,7 +1200,7 @@ describe('MDC-based MatDialog', () => {
12001200
expect(document.activeElement!.id)
12011201
.not.toBe(
12021202
'dialog-trigger',
1203-
'Expcted the focus not to have changed before the animation finishes.');
1203+
'Expected the focus not to have changed before the animation finishes.');
12041204

12051205
flushMicrotasks();
12061206
viewContainerFixture.detectChanges();
@@ -1929,6 +1929,14 @@ describe('MDC-based MatDialog with animations enabled', () => {
19291929
flush();
19301930
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
19311931
}));
1932+
1933+
it('should return the previous dialogRef if the previous dialog hasn\'t finished animating open',
1934+
() => {
1935+
let dialogRef1: MatDialogRef<PizzaMsg>, dialogRef2: MatDialogRef<PizzaMsg>;
1936+
dialogRef1 = dialog.open(PizzaMsg);
1937+
dialogRef2 = dialog.open(PizzaMsg);
1938+
expect(dialogRef1).toEqual(dialogRef2);
1939+
});
19321940
});
19331941

19341942
@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: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
114114
// Save the previously focused element. This element will be re-focused
115115
// when the dialog closes.
116116
this._capturePreviouslyFocusedElement();
117-
// Move focus onto the dialog immediately in order to prevent the user
118-
// from accidentally opening multiple dialogs at the same time.
119-
this._focusDialogContainer();
120117
}
121118

122119
/**
@@ -218,7 +215,13 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
218215
break;
219216
case true:
220217
case 'first-tabbable':
221-
this._focusTrap.focusInitialElementWhenReady();
218+
this._focusTrap.focusInitialElementWhenReady().then(focusedSuccessfully => {
219+
// If we weren't able to find a focusable element in the dialog, then focus the dialog
220+
// container instead.
221+
if (!focusedSuccessfully) {
222+
this._focusDialogContainer();
223+
}
224+
});
222225
break;
223226
case 'first-heading':
224227
this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]');

src/material/dialog/dialog.spec.ts

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

13351335
tick(500);
13361336
viewContainerFixture.detectChanges();
1337-
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();
1337+
expect(lastFocusOrigin!).toBe('program');
13381338

13391339
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
13401340

@@ -1367,7 +1367,7 @@ describe('MatDialog', () => {
13671367

13681368
tick(500);
13691369
viewContainerFixture.detectChanges();
1370-
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();
1370+
expect(lastFocusOrigin!).toBe('program');
13711371

13721372
const backdrop = overlayContainerElement
13731373
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
@@ -1398,12 +1398,18 @@ describe('MatDialog', () => {
13981398
// Patch the element focus after the initial and real focus, because otherwise the
13991399
// `activeElement` won't be set, and the dialog won't be able to restore focus to an element.
14001400
patchElementFocus(button);
1401+
expect(lastFocusOrigin!)
1402+
.withContext('Expected the trigger button to be focused via program')
1403+
.toBe('program');
14011404

14021405
dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef});
14031406

14041407
tick(500);
14051408
viewContainerFixture.detectChanges();
1406-
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();
1409+
tick(500);
1410+
expect(lastFocusOrigin!)
1411+
.withContext('Expected the trigger button to be blurred')
1412+
.toBeNull();
14071413

14081414
const closeButton = overlayContainerElement
14091415
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1416,7 +1422,8 @@ describe('MatDialog', () => {
14161422
tick(500);
14171423

14181424
expect(lastFocusOrigin!)
1419-
.withContext('Expected the trigger button to be focused via keyboard').toBe('keyboard');
1425+
.withContext( 'Expected the trigger button to be focused via keyboard')
1426+
.toBe('keyboard');
14201427

14211428
focusMonitor.stopMonitoring(button);
14221429
document.body.removeChild(button);
@@ -1435,12 +1442,17 @@ describe('MatDialog', () => {
14351442
// Patch the element focus after the initial and real focus, because otherwise the
14361443
// `activeElement` won't be set, and the dialog won't be able to restore focus to an element.
14371444
patchElementFocus(button);
1445+
expect(lastFocusOrigin!)
1446+
.withContext('Expected the trigger button to be focused via program')
1447+
.toBe('program');
14381448

14391449
dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef});
14401450

1441-
tick(500);
14421451
viewContainerFixture.detectChanges();
1443-
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();
1452+
tick(500);
1453+
expect(lastFocusOrigin!)
1454+
.withContext('Expected the trigger button to be blurred')
1455+
.toBeNull();
14441456

14451457
const closeButton = overlayContainerElement
14461458
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1454,7 +1466,8 @@ describe('MatDialog', () => {
14541466
tick(500);
14551467

14561468
expect(lastFocusOrigin!)
1457-
.withContext('Expected the trigger button to be focused via mouse').toBe('mouse');
1469+
.withContext( 'Expected the trigger button to be focused via mouse')
1470+
.toBe('mouse');
14581471

14591472
focusMonitor.stopMonitoring(button);
14601473
document.body.removeChild(button);

src/material/dialog/dialog.ts

Lines changed: 39 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,9 @@ 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;
83+
private _lastDialogRef: MatDialogRef<any>;
8084

8185
/** Keeps track of the currently-open dialogs. */
8286
get openDialogs(): MatDialogRef<any>[] {
@@ -111,7 +115,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
111115
scrollStrategy: any,
112116
private _dialogRefConstructor: Type<MatDialogRef<any>>,
113117
private _dialogContainerType: Type<C>,
114-
private _dialogDataToken: InjectionToken<any>) {
118+
private _dialogDataToken: InjectionToken<any>,
119+
private _animationMode?: 'NoopAnimations' | 'BrowserAnimations') {
115120
this._scrollStrategy = scrollStrategy;
116121
}
117122

@@ -145,12 +150,35 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
145150
throw Error(`Dialog with id "${config.id}" exists already. The dialog id must be unique.`);
146151
}
147152

153+
// If there is a dialog that is currently animating open, return the MatDialogRef of that dialog
154+
if (this._dialogAnimatingOpen) {
155+
return this._lastDialogRef;
156+
}
157+
148158
const overlayRef = this._createOverlay(config);
149159
const dialogContainer = this._attachDialogContainer(overlayRef, config);
160+
if (this._animationMode !== 'NoopAnimations') {
161+
const animationStateSubscription =
162+
dialogContainer._animationStateChanged.subscribe((dialogAnimationEvent) => {
163+
if (dialogAnimationEvent.state === 'opening') {
164+
this._dialogAnimatingOpen = true;
165+
}
166+
if (dialogAnimationEvent.state === 'opened') {
167+
this._dialogAnimatingOpen = false;
168+
animationStateSubscription.unsubscribe();
169+
}
170+
});
171+
if (!this._animationStateSubscriptions) {
172+
this._animationStateSubscriptions = new Subscription();
173+
}
174+
this._animationStateSubscriptions.add(animationStateSubscription);
175+
}
176+
150177
const dialogRef = this._attachDialogContent<T, R>(componentOrTemplateRef,
151178
dialogContainer,
152179
overlayRef,
153180
config);
181+
this._lastDialogRef = dialogRef;
154182

155183
// If this is the first dialog that we're opening, hide all the non-overlay content.
156184
if (!this.openDialogs.length) {
@@ -188,6 +216,10 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
188216
this._closeDialogs(this._openDialogsAtThisLevel);
189217
this._afterAllClosedAtThisLevel.complete();
190218
this._afterOpenedAtThisLevel.complete();
219+
// Clean up any subscriptions to dialogs that never finished opening.
220+
if (this._animationStateSubscriptions) {
221+
this._animationStateSubscriptions.unsubscribe();
222+
}
191223
}
192224

193225
/**
@@ -392,19 +424,19 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
392424
@Injectable()
393425
export class MatDialog extends _MatDialogBase<MatDialogContainer> {
394426
constructor(
395-
overlay: Overlay,
396-
injector: Injector,
427+
overlay: Overlay, injector: Injector,
397428
/**
398429
* @deprecated `_location` parameter to be removed.
399430
* @breaking-change 10.0.0
400431
*/
401432
@Optional() location: Location,
402433
@Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig,
403434
@Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
404-
@Optional() @SkipSelf() parentDialog: MatDialog,
405-
overlayContainer: OverlayContainer) {
435+
@Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer,
436+
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: 'NoopAnimations'|
437+
'BrowserAnimations') {
406438
super(overlay, injector, defaultOptions, parentDialog, overlayContainer, scrollStrategy,
407-
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA);
439+
MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA, animationMode);
408440
}
409441
}
410442

tools/public_api_guard/material/dialog.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ export function MAT_DIALOG_SCROLL_STRATEGY_PROVIDER_FACTORY(overlay: Overlay): (
8787
// @public
8888
export class MatDialog extends _MatDialogBase<MatDialogContainer> {
8989
constructor(overlay: Overlay, injector: Injector,
90-
location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer);
90+
location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer, animationMode?: 'NoopAnimations' | 'BrowserAnimations');
9191
// (undocumented)
92-
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialog, [null, null, { optional: true; }, { optional: true; }, null, { optional: true; skipSelf: true; }, null]>;
92+
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialog, [null, null, { optional: true; }, { optional: true; }, null, { optional: true; skipSelf: true; }, null, { optional: true; }]>;
9393
// (undocumented)
9494
static ɵprov: i0.ɵɵInjectableDeclaration<MatDialog>;
9595
}
@@ -109,7 +109,7 @@ export const matDialogAnimations: {
109109

110110
// @public
111111
export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implements OnDestroy {
112-
constructor(_overlay: Overlay, _injector: Injector, _defaultOptions: MatDialogConfig | undefined, _parentDialog: _MatDialogBase<C> | undefined, _overlayContainer: OverlayContainer, scrollStrategy: any, _dialogRefConstructor: Type<MatDialogRef<any>>, _dialogContainerType: Type<C>, _dialogDataToken: InjectionToken<any>);
112+
constructor(_overlay: Overlay, _injector: Injector, _defaultOptions: MatDialogConfig | undefined, _parentDialog: _MatDialogBase<C> | undefined, _overlayContainer: OverlayContainer, scrollStrategy: any, _dialogRefConstructor: Type<MatDialogRef<any>>, _dialogContainerType: Type<C>, _dialogDataToken: InjectionToken<any>, _animationMode?: "NoopAnimations" | "BrowserAnimations" | undefined);
113113
readonly afterAllClosed: Observable<void>;
114114
get afterOpened(): Subject<MatDialogRef<any>>;
115115
closeAll(): void;

0 commit comments

Comments
 (0)