Skip to content

Commit 235b0bc

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 2bd5fab commit 235b0bc

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
@@ -772,14 +772,14 @@ describe('MDC-based MatDialog', () => {
772772
expect(dialogRef.componentInstance).toBeFalsy('Expected reference to have been cleared.');
773773
}));
774774

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

779779
expect(one.id).toBeTruthy();
780780
expect(two.id).toBeTruthy();
781781
expect(one.id).not.toBe(two.id);
782-
});
782+
}));
783783

784784
it('should allow for the id to be overwritten', () => {
785785
const dialogRef = dialog.open(PizzaMsg, {id: 'pizza'});
@@ -1188,7 +1188,7 @@ describe('MDC-based MatDialog', () => {
11881188
expect(document.activeElement!.id)
11891189
.not.toBe(
11901190
'dialog-trigger',
1191-
'Expcted the focus not to have changed before the animation finishes.');
1191+
'Expected the focus not to have changed before the animation finishes.');
11921192

11931193
flushMicrotasks();
11941194
viewContainerFixture.detectChanges();
@@ -1913,6 +1913,14 @@ describe('MDC-based MatDialog with animations enabled', () => {
19131913
flush();
19141914
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
19151915
}));
1916+
1917+
it('should return the previous dialogRef if the previous dialog hasn\'t finished animating open',
1918+
() => {
1919+
let dialogRef1: MatDialogRef<PizzaMsg>, dialogRef2: MatDialogRef<PizzaMsg>;
1920+
dialogRef1 = dialog.open(PizzaMsg);
1921+
dialogRef2 = dialog.open(PizzaMsg);
1922+
expect(dialogRef1).toEqual(dialogRef2);
1923+
});
19161924
});
19171925

19181926
@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
@@ -1320,7 +1320,7 @@ describe('MatDialog', () => {
13201320

13211321
tick(500);
13221322
viewContainerFixture.detectChanges();
1323-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1323+
expect(lastFocusOrigin!).toBe('program');
13241324

13251325
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
13261326

@@ -1353,7 +1353,7 @@ describe('MatDialog', () => {
13531353

13541354
tick(500);
13551355
viewContainerFixture.detectChanges();
1356-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1356+
expect(lastFocusOrigin!).toBe('program');
13571357

13581358
const backdrop = overlayContainerElement
13591359
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
@@ -1384,12 +1384,18 @@ describe('MatDialog', () => {
13841384
// Patch the element focus after the initial and real focus, because otherwise the
13851385
// `activeElement` won't be set, and the dialog won't be able to restore focus to an element.
13861386
patchElementFocus(button);
1387+
expect(lastFocusOrigin!)
1388+
.withContext('Expected the trigger button to be focused via program')
1389+
.toBe('program');
13871390

13881391
dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef});
13891392

13901393
tick(500);
13911394
viewContainerFixture.detectChanges();
1392-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1395+
tick(500);
1396+
expect(lastFocusOrigin!)
1397+
.withContext('Expected the trigger button to be blurred')
1398+
.toBeNull();
13931399

13941400
const closeButton = overlayContainerElement
13951401
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1402,7 +1408,8 @@ describe('MatDialog', () => {
14021408
tick(500);
14031409

14041410
expect(lastFocusOrigin!)
1405-
.toBe('keyboard', 'Expected the trigger button to be focused via keyboard');
1411+
.withContext( 'Expected the trigger button to be focused via keyboard')
1412+
.toBe('keyboard');
14061413

14071414
focusMonitor.stopMonitoring(button);
14081415
document.body.removeChild(button);
@@ -1421,12 +1428,17 @@ describe('MatDialog', () => {
14211428
// Patch the element focus after the initial and real focus, because otherwise the
14221429
// `activeElement` won't be set, and the dialog won't be able to restore focus to an element.
14231430
patchElementFocus(button);
1431+
expect(lastFocusOrigin!)
1432+
.withContext('Expected the trigger button to be focused via program')
1433+
.toBe('program');
14241434

14251435
dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef});
14261436

1427-
tick(500);
14281437
viewContainerFixture.detectChanges();
1429-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1438+
tick(500);
1439+
expect(lastFocusOrigin!)
1440+
.withContext('Expected the trigger button to be blurred')
1441+
.toBeNull();
14301442

14311443
const closeButton = overlayContainerElement
14321444
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1440,7 +1452,8 @@ describe('MatDialog', () => {
14401452
tick(500);
14411453

14421454
expect(lastFocusOrigin!)
1443-
.toBe('mouse', 'Expected the trigger button to be focused via mouse');
1455+
.withContext( 'Expected the trigger button to be focused via mouse')
1456+
.toBe('mouse');
14441457

14451458
focusMonitor.stopMonitoring(button);
14461459
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)