fix(material/dialog): don't wait for animation before moving focus

For a long time we used to move focus into the dialog after the opening animation was finished, because it allowed for the content to settle in case something like an autocomplete needed to open. This approach has caused us accessibility issues in the past, because we've had to find ways to prevent the user from interacting with content behind the dialog while the opening animation is running.

Initially we used to move focus to the dialog container before transferring it into the dialog, but that ended up interrupting the screen reader. In a later change we started returning the previous dialog ref if the same kind of dialog is opened while the animation is still running. The problem with this approach is that it doesn't allow for multiple dialog to be opened quickly (see #24037).

These changes aim to address the root cause by moving focus immediately after the dialog content is attached. If this causes regressions with autocompletes inside dialogs, we now have APIs that allow users to customize the focus behavior.

Fixes #24037.

Author: crisbeto

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

@@ -2005,13 +2005,6 @@ 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<PizzaMsg>, dialogRef2: MatDialogRef<PizzaMsg>;
-    dialogRef1 = dialog.open(PizzaMsg);
-    dialogRef2 = dialog.open(PizzaMsg);
-    expect(dialogRef1).toEqual(dialogRef2);
-  });
 });
 
 @Directive({selector: 'dir-with-view-container'})

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

@@ -58,6 +58,10 @@ export class MatDialog extends _MatDialogBase<MatDialogContainer> {
     @Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
     @Optional() @SkipSelf() parentDialog: MatDialog,
     overlayContainer: OverlayContainer,
+    /**
+     * @deprecated No longer used. To be removed.
+     * @breaking-change 14.0.0
+     */
     @Optional()
     @Inject(ANIMATION_MODULE_TYPE)
     animationMode?: 'NoopAnimations' | 'BrowserAnimations',

src/material/dialog/dialog-container.ts

@@ -110,10 +110,15 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
 
   /** Initializes the dialog container with the attached content. */
   _initializeWithAttachedContent() {
-    this._setupFocusTrap();
+    this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement);
+
     // Save the previously focused element. This element will be re-focused
     // when the dialog closes.
-    this._capturePreviouslyFocusedElement();
+    if (this._document) {
+      this._elementFocusedBeforeDialogWasOpened = _getFocusedElementPierceShadowDom();
+    }
+
+    this._trapFocus();
   }
 
   /**
@@ -270,18 +275,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
     }
   }
 
-  /** Sets up the focus trap. */
-  private _setupFocusTrap() {
-    this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement);
-  }
-
-  /** Captures the element that was focused before the dialog was opened. */
-  private _capturePreviouslyFocusedElement() {
-    if (this._document) {
-      this._elementFocusedBeforeDialogWasOpened = _getFocusedElementPierceShadowDom();
-    }
-  }
-
   /** Focuses the dialog container. */
   private _focusDialogContainer() {
     // Note that there is no focus method when rendering on the server.
@@ -333,7 +326,6 @@ export class MatDialogContainer extends _MatDialogContainerBase {
   /** Callback, invoked whenever an animation on the host completes. */
   _onAnimationDone({toState, totalTime}: AnimationEvent) {
     if (toState === 'enter') {
-      this._trapFocus();
       this._animationStateChanged.next({state: 'opened', totalTime});
     } else if (toState === 'exit') {
       this._restoreFocus();

src/material/dialog/dialog.spec.ts

@@ -1388,7 +1388,7 @@ describe('MatDialog', () => {
 
       tick(500);
       viewContainerFixture.detectChanges();
-      expect(lastFocusOrigin!).toBe('program');
+      expect(lastFocusOrigin).toBe(null);
 
       dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
 
@@ -1421,7 +1421,7 @@ describe('MatDialog', () => {
 
       tick(500);
       viewContainerFixture.detectChanges();
-      expect(lastFocusOrigin!).toBe('program');
+      expect(lastFocusOrigin).toBe(null);
 
       const backdrop = overlayContainerElement.querySelector(
         '.cdk-overlay-backdrop',

src/material/dialog/dialog.ts

@@ -30,7 +30,7 @@ import {
   TemplateRef,
   Type,
 } from '@angular/core';
-import {defer, Observable, of as observableOf, Subject, Subscription} from 'rxjs';
+import {defer, Observable, of as observableOf, Subject} from 'rxjs';
 import {startWith} from 'rxjs/operators';
 import {MatDialogConfig} from './dialog-config';
 import {MatDialogContainer, _MatDialogContainerBase} from './dialog-container';
@@ -80,9 +80,6 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
   private readonly _afterOpenedAtThisLevel = new Subject<MatDialogRef<any>>();
   private _ariaHiddenElements = new Map<Element, string | null>();
   private _scrollStrategy: () => ScrollStrategy;
-  private _dialogAnimatingOpen = false;
-  private _animationStateSubscriptions: Subscription;
-  private _lastDialogRef: MatDialogRef<any>;
 
   /** Keeps track of the currently-open dialogs. */
   get openDialogs(): MatDialogRef<any>[] {
@@ -120,7 +117,11 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
     private _dialogRefConstructor: Type<MatDialogRef<any>>,
     private _dialogContainerType: Type<C>,
     private _dialogDataToken: InjectionToken<any>,
-    private _animationMode?: 'NoopAnimations' | 'BrowserAnimations',
+    /**
+     * @deprecated No longer used. To be removed.
+     * @breaking-change 14.0.0
+     */
+    _animationMode?: 'NoopAnimations' | 'BrowserAnimations',
   ) {
     this._scrollStrategy = scrollStrategy;
   }
@@ -166,38 +167,14 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> 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<T, R>(
       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) {
@@ -235,10 +212,6 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> 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();
-    }
   }
 
   /**
@@ -463,6 +436,10 @@ export class MatDialog extends _MatDialogBase<MatDialogContainer> {
     @Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
     @Optional() @SkipSelf() parentDialog: MatDialog,
     overlayContainer: OverlayContainer,
+    /**
+     * @deprecated No longer used. To be removed.
+     * @breaking-change 14.0.0
+     */
     @Optional()
     @Inject(ANIMATION_MODULE_TYPE)
     animationMode?: 'NoopAnimations' | 'BrowserAnimations',

tools/public_api_guard/material/dialog.md

@@ -87,7 +87,8 @@ export function MAT_DIALOG_SCROLL_STRATEGY_PROVIDER_FACTORY(overlay: Overlay): (
 // @public
 export class MatDialog extends _MatDialogBase<MatDialogContainer> {
     constructor(overlay: Overlay, injector: Injector,
-    location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer, animationMode?: 'NoopAnimations' | 'BrowserAnimations');
+    location: Location_2, defaultOptions: MatDialogConfig, scrollStrategy: any, parentDialog: MatDialog, overlayContainer: OverlayContainer,
+    animationMode?: 'NoopAnimations' | 'BrowserAnimations');
     // (undocumented)
     static ɵfac: i0.ɵɵFactoryDeclaration<MatDialog, [null, null, { optional: true; }, { optional: true; }, null, { optional: true; skipSelf: true; }, null, { optional: true; }]>;
     // (undocumented)
@@ -109,7 +110,8 @@ export const matDialogAnimations: {
 
 // @public
 export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implements OnDestroy {
-    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);
+    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');
     readonly afterAllClosed: Observable<void>;
     get afterOpened(): Subject<MatDialogRef<any>>;
     closeAll(): void;