From 5dd05036e83acaef557e560c2d73e1af6626a5a3 Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Mon, 28 Jun 2021 22:12:02 -0400 Subject: [PATCH] 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 --- .../mdc-dialog/dialog.spec.ts | 14 ++++-- .../mdc-dialog/dialog.ts | 7 ++- src/material/dialog/dialog-container.ts | 11 +++-- src/material/dialog/dialog.spec.ts | 27 ++++++++--- src/material/dialog/dialog.ts | 46 ++++++++++++++++--- tools/public_api_guard/material/dialog.md | 6 +-- 6 files changed, 85 insertions(+), 26 deletions(-) diff --git a/src/material-experimental/mdc-dialog/dialog.spec.ts b/src/material-experimental/mdc-dialog/dialog.spec.ts index 93c735dcc0f2..3c805905f78d 100644 --- a/src/material-experimental/mdc-dialog/dialog.spec.ts +++ b/src/material-experimental/mdc-dialog/dialog.spec.ts @@ -774,14 +774,14 @@ describe('MDC-based MatDialog', () => { .withContext('Expected reference to have been cleared.').toBeFalsy(); })); - it('should assign a unique id to each dialog', () => { + it('should assign a unique id to each dialog', fakeAsync(() => { const one = dialog.open(PizzaMsg); const two = dialog.open(PizzaMsg); expect(one.id).toBeTruthy(); expect(two.id).toBeTruthy(); expect(one.id).not.toBe(two.id); - }); + })); it('should allow for the id to be overwritten', () => { const dialogRef = dialog.open(PizzaMsg, {id: 'pizza'}); @@ -1200,7 +1200,7 @@ describe('MDC-based MatDialog', () => { expect(document.activeElement!.id) .not.toBe( 'dialog-trigger', - 'Expcted the focus not to have changed before the animation finishes.'); + 'Expected the focus not to have changed before the animation finishes.'); flushMicrotasks(); viewContainerFixture.detectChanges(); @@ -1929,6 +1929,14 @@ describe('MDC-based MatDialog with animations enabled', () => { flush(); expect(dialogRef.getState()).toBe(MatDialogState.CLOSED); })); + + it('should return the previous dialogRef if the previous dialog hasn\'t finished animating open', + () => { + let dialogRef1: MatDialogRef, dialogRef2: MatDialogRef; + dialogRef1 = dialog.open(PizzaMsg); + dialogRef2 = dialog.open(PizzaMsg); + expect(dialogRef1).toEqual(dialogRef2); + }); }); @Directive({selector: 'dir-with-view-container'}) diff --git a/src/material-experimental/mdc-dialog/dialog.ts b/src/material-experimental/mdc-dialog/dialog.ts index d27b61434fea..11f40955b42d 100644 --- a/src/material-experimental/mdc-dialog/dialog.ts +++ b/src/material-experimental/mdc-dialog/dialog.ts @@ -12,6 +12,7 @@ import {Inject, Injectable, InjectionToken, Injector, Optional, SkipSelf} from ' import {_MatDialogBase, MatDialogConfig} from '@angular/material/dialog'; import {MatDialogContainer} from './dialog-container'; import {MatDialogRef} from './dialog-ref'; +import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; /** Injection token that can be used to access the data that was passed in to a dialog. */ export const MAT_DIALOG_DATA = new InjectionToken('MatMdcDialogData'); @@ -52,9 +53,11 @@ export class MatDialog extends _MatDialogBase { @Optional() location: Location, @Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig, @Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any, - @Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer) { + @Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer, + @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: 'NoopAnimations'| + 'BrowserAnimations') { super( overlay, injector, defaultOptions, parentDialog, overlayContainer, scrollStrategy, - MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA); + MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA, animationMode); } } diff --git a/src/material/dialog/dialog-container.ts b/src/material/dialog/dialog-container.ts index e66bae7eec05..4c1946ac2971 100644 --- a/src/material/dialog/dialog-container.ts +++ b/src/material/dialog/dialog-container.ts @@ -114,9 +114,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet { // Save the previously focused element. This element will be re-focused // when the dialog closes. this._capturePreviouslyFocusedElement(); - // Move focus onto the dialog immediately in order to prevent the user - // from accidentally opening multiple dialogs at the same time. - this._focusDialogContainer(); } /** @@ -218,7 +215,13 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet { break; case true: case 'first-tabbable': - this._focusTrap.focusInitialElementWhenReady(); + this._focusTrap.focusInitialElementWhenReady().then(focusedSuccessfully => { + // If we weren't able to find a focusable element in the dialog, then focus the dialog + // container instead. + if (!focusedSuccessfully) { + this._focusDialogContainer(); + } + }); break; case 'first-heading': this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]'); diff --git a/src/material/dialog/dialog.spec.ts b/src/material/dialog/dialog.spec.ts index a9209498290f..ffcf9142f048 100644 --- a/src/material/dialog/dialog.spec.ts +++ b/src/material/dialog/dialog.spec.ts @@ -1334,7 +1334,7 @@ describe('MatDialog', () => { tick(500); viewContainerFixture.detectChanges(); - expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull(); + expect(lastFocusOrigin!).toBe('program'); dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); @@ -1367,7 +1367,7 @@ describe('MatDialog', () => { tick(500); viewContainerFixture.detectChanges(); - expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull(); + expect(lastFocusOrigin!).toBe('program'); const backdrop = overlayContainerElement .querySelector('.cdk-overlay-backdrop') as HTMLElement; @@ -1398,12 +1398,18 @@ describe('MatDialog', () => { // Patch the element focus after the initial and real focus, because otherwise the // `activeElement` won't be set, and the dialog won't be able to restore focus to an element. patchElementFocus(button); + expect(lastFocusOrigin!) + .withContext('Expected the trigger button to be focused via program') + .toBe('program'); dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef}); tick(500); viewContainerFixture.detectChanges(); - expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull(); + tick(500); + expect(lastFocusOrigin!) + .withContext('Expected the trigger button to be blurred') + .toBeNull(); const closeButton = overlayContainerElement .querySelector('button[mat-dialog-close]') as HTMLElement; @@ -1416,7 +1422,8 @@ describe('MatDialog', () => { tick(500); expect(lastFocusOrigin!) - .withContext('Expected the trigger button to be focused via keyboard').toBe('keyboard'); + .withContext( 'Expected the trigger button to be focused via keyboard') + .toBe('keyboard'); focusMonitor.stopMonitoring(button); document.body.removeChild(button); @@ -1435,12 +1442,17 @@ describe('MatDialog', () => { // Patch the element focus after the initial and real focus, because otherwise the // `activeElement` won't be set, and the dialog won't be able to restore focus to an element. patchElementFocus(button); + expect(lastFocusOrigin!) + .withContext('Expected the trigger button to be focused via program') + .toBe('program'); dialog.open(ContentElementDialog, {viewContainerRef: testViewContainerRef}); - tick(500); viewContainerFixture.detectChanges(); - expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull(); + tick(500); + expect(lastFocusOrigin!) + .withContext('Expected the trigger button to be blurred') + .toBeNull(); const closeButton = overlayContainerElement .querySelector('button[mat-dialog-close]') as HTMLElement; @@ -1454,7 +1466,8 @@ describe('MatDialog', () => { tick(500); expect(lastFocusOrigin!) - .withContext('Expected the trigger button to be focused via mouse').toBe('mouse'); + .withContext( 'Expected the trigger button to be focused via mouse') + .toBe('mouse'); focusMonitor.stopMonitoring(button); document.body.removeChild(button); diff --git a/src/material/dialog/dialog.ts b/src/material/dialog/dialog.ts index a9e4d75ad845..ab3d677d5535 100644 --- a/src/material/dialog/dialog.ts +++ b/src/material/dialog/dialog.ts @@ -30,11 +30,12 @@ import { TemplateRef, Type, } from '@angular/core'; -import {defer, Observable, of as observableOf, Subject} from 'rxjs'; +import {defer, Observable, of as observableOf, Subject, Subscription} from 'rxjs'; import {startWith} from 'rxjs/operators'; import {MatDialogConfig} from './dialog-config'; import {MatDialogContainer, _MatDialogContainerBase} from './dialog-container'; import {MatDialogRef} from './dialog-ref'; +import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; /** 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 implemen private readonly _afterOpenedAtThisLevel = new Subject>(); private _ariaHiddenElements = new Map(); private _scrollStrategy: () => ScrollStrategy; + private _dialogAnimatingOpen = false; + private _animationStateSubscriptions: Subscription; + private _lastDialogRef: MatDialogRef; /** Keeps track of the currently-open dialogs. */ get openDialogs(): MatDialogRef[] { @@ -111,7 +115,8 @@ export abstract class _MatDialogBase implemen scrollStrategy: any, private _dialogRefConstructor: Type>, private _dialogContainerType: Type, - private _dialogDataToken: InjectionToken) { + private _dialogDataToken: InjectionToken, + private _animationMode?: 'NoopAnimations' | 'BrowserAnimations') { this._scrollStrategy = scrollStrategy; } @@ -145,12 +150,35 @@ export abstract class _MatDialogBase implemen throw Error(`Dialog with id "${config.id}" exists already. The dialog id must be unique.`); } + // If there is a dialog that is currently animating open, return the MatDialogRef of that dialog + if (this._dialogAnimatingOpen) { + return this._lastDialogRef; + } + const overlayRef = this._createOverlay(config); const dialogContainer = this._attachDialogContainer(overlayRef, config); + if (this._animationMode !== 'NoopAnimations') { + const animationStateSubscription = + dialogContainer._animationStateChanged.subscribe((dialogAnimationEvent) => { + if (dialogAnimationEvent.state === 'opening') { + this._dialogAnimatingOpen = true; + } + if (dialogAnimationEvent.state === 'opened') { + this._dialogAnimatingOpen = false; + animationStateSubscription.unsubscribe(); + } + }); + if (!this._animationStateSubscriptions) { + this._animationStateSubscriptions = new Subscription(); + } + this._animationStateSubscriptions.add(animationStateSubscription); + } + const dialogRef = this._attachDialogContent(componentOrTemplateRef, dialogContainer, overlayRef, config); + this._lastDialogRef = dialogRef; // If this is the first dialog that we're opening, hide all the non-overlay content. if (!this.openDialogs.length) { @@ -188,6 +216,10 @@ export abstract class _MatDialogBase implemen this._closeDialogs(this._openDialogsAtThisLevel); this._afterAllClosedAtThisLevel.complete(); this._afterOpenedAtThisLevel.complete(); + // Clean up any subscriptions to dialogs that never finished opening. + if (this._animationStateSubscriptions) { + this._animationStateSubscriptions.unsubscribe(); + } } /** @@ -392,8 +424,7 @@ export abstract class _MatDialogBase implemen @Injectable() export class MatDialog extends _MatDialogBase { constructor( - overlay: Overlay, - injector: Injector, + overlay: Overlay, injector: Injector, /** * @deprecated `_location` parameter to be removed. * @breaking-change 10.0.0 @@ -401,10 +432,11 @@ export class MatDialog extends _MatDialogBase { @Optional() location: Location, @Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig, @Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any, - @Optional() @SkipSelf() parentDialog: MatDialog, - overlayContainer: OverlayContainer) { + @Optional() @SkipSelf() parentDialog: MatDialog, overlayContainer: OverlayContainer, + @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: 'NoopAnimations'| + 'BrowserAnimations') { super(overlay, injector, defaultOptions, parentDialog, overlayContainer, scrollStrategy, - MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA); + MatDialogRef, MatDialogContainer, MAT_DIALOG_DATA, animationMode); } } diff --git a/tools/public_api_guard/material/dialog.md b/tools/public_api_guard/material/dialog.md index 31e78075329c..35da0a953e73 100644 --- a/tools/public_api_guard/material/dialog.md +++ b/tools/public_api_guard/material/dialog.md @@ -87,9 +87,9 @@ export function MAT_DIALOG_SCROLL_STRATEGY_PROVIDER_FACTORY(overlay: Overlay): ( // @public export class MatDialog extends _MatDialogBase { constructor(overlay: Overlay, injector: Injector, - location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer); + location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer, animationMode?: 'NoopAnimations' | 'BrowserAnimations'); // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: i0.ɵɵFactoryDeclaration; // (undocumented) static ɵprov: i0.ɵɵInjectableDeclaration; } @@ -109,7 +109,7 @@ export const matDialogAnimations: { // @public export abstract class _MatDialogBase implements OnDestroy { - constructor(_overlay: Overlay, _injector: Injector, _defaultOptions: MatDialogConfig | undefined, _parentDialog: _MatDialogBase | undefined, _overlayContainer: OverlayContainer, scrollStrategy: any, _dialogRefConstructor: Type>, _dialogContainerType: Type, _dialogDataToken: InjectionToken); + constructor(_overlay: Overlay, _injector: Injector, _defaultOptions: MatDialogConfig | undefined, _parentDialog: _MatDialogBase | undefined, _overlayContainer: OverlayContainer, scrollStrategy: any, _dialogRefConstructor: Type>, _dialogContainerType: Type, _dialogDataToken: InjectionToken, _animationMode?: "NoopAnimations" | "BrowserAnimations" | undefined); readonly afterAllClosed: Observable; get afterOpened(): Subject>; closeAll(): void;