Skip to content

Commit 0e636aa

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 0e636aa

File tree

4 files changed

+54
-13
lines changed

4 files changed

+54
-13
lines changed

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

Lines changed: 20 additions & 2 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,24 @@ 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+
}));
792+
793+
it('should not allow another dialog to open until the previous dialog finished animating open',
794+
() => {
795+
const openTwoDialogs = () => {
796+
dialog.open(PizzaMsg);
797+
dialog.open(PizzaMsg);
798+
};
799+
expect(openTwoDialogs).toThrowError(/Ignoring attempt to open dialog/);
800+
});
783801

784802
it('should allow for the id to be overwritten', () => {
785803
const dialogRef = dialog.open(PizzaMsg, {id: 'pizza'});

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: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ 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';
@@ -77,6 +77,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
7777
private readonly _afterOpenedAtThisLevel = new Subject<MatDialogRef<any>>();
7878
private _ariaHiddenElements = new Map<Element, string|null>();
7979
private _scrollStrategy: () => ScrollStrategy;
80+
private _dialogAnimatingOpen = false;
81+
private _animationStateSubscriptions = new Subscription();
8082

8183
/** Keeps track of the currently-open dialogs. */
8284
get openDialogs(): MatDialogRef<any>[] {
@@ -144,9 +146,25 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
144146
(typeof ngDevMode === 'undefined' || ngDevMode)) {
145147
throw Error(`Dialog with id "${config.id}" exists already. The dialog id must be unique.`);
146148
}
149+
if (this._dialogAnimatingOpen) {
150+
throw Error(`Ignoring attempt to open dialog with id "${
151+
config.id}" since another dialog is currently animating open.`);
152+
}
147153

148154
const overlayRef = this._createOverlay(config);
149155
const dialogContainer = this._attachDialogContainer(overlayRef, config);
156+
const animationStateSubscription =
157+
dialogContainer._animationStateChanged.subscribe((dialogAnimationEvent) => {
158+
if (dialogAnimationEvent.state === 'opening') {
159+
this._dialogAnimatingOpen = true;
160+
}
161+
if (dialogAnimationEvent.state === 'opened') {
162+
this._dialogAnimatingOpen = false;
163+
animationStateSubscription.unsubscribe();
164+
}
165+
});
166+
this._animationStateSubscriptions.add(animationStateSubscription);
167+
150168
const dialogRef = this._attachDialogContent<T, R>(componentOrTemplateRef,
151169
dialogContainer,
152170
overlayRef,
@@ -188,6 +206,8 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
188206
this._closeDialogs(this._openDialogsAtThisLevel);
189207
this._afterAllClosedAtThisLevel.complete();
190208
this._afterOpenedAtThisLevel.complete();
209+
// Clean up any subscriptions to dialogs that never finished opening.
210+
this._animationStateSubscriptions.unsubscribe();
191211
}
192212

193213
/**

0 commit comments

Comments
 (0)