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

…24121) * 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. * fixup! fix(material/dialog): don't wait for animation before moving focus (cherry picked from commit 92863cc)
angular · Mar 7, 2022 · c47d30e · c47d30e
1 parent 4b5363d
commit c47d30e
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 57 deletions.
diff --git a/src/material-experimental/mdc-dialog/dialog.spec.ts b/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'})

diff --git a/src/material-experimental/mdc-dialog/dialog.ts b/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',

diff --git a/src/material/dialog/dialog-config.ts b/src/material/dialog/dialog-config.ts
@@ -110,6 +110,9 @@ export class MatDialogConfig<D = any> {
    */
   restoreFocus?: boolean = true;
 
+  /** Whether to wait for the opening animation to finish before trapping focus. */
+  delayFocusTrap?: boolean = true;
+
   /** Scroll strategy to be used for the dialog. */
   scrollStrategy?: ScrollStrategy;
 

diff --git a/src/material/dialog/dialog-container.ts b/src/material/dialog/dialog-container.ts
@@ -110,10 +110,13 @@ 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();
+    }
   }
 
   /**
@@ -270,18 +273,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 +324,10 @@ export class MatDialogContainer extends _MatDialogContainerBase {
   /** Callback, invoked whenever an animation on the host completes. */
   _onAnimationDone({toState, totalTime}: AnimationEvent) {
     if (toState === 'enter') {
-      this._trapFocus();
+      if (this._config.delayFocusTrap) {
+        this._trapFocus();
+      }
+
       this._animationStateChanged.next({state: 'opened', totalTime});
     } else if (toState === 'exit') {
       this._restoreFocus();
@@ -358,4 +352,12 @@ export class MatDialogContainer extends _MatDialogContainerBase {
     // view container is using OnPush change detection.
     this._changeDetectorRef.markForCheck();
   }
+
+  override _initializeWithAttachedContent() {
+    super._initializeWithAttachedContent();
+
+    if (!this._config.delayFocusTrap) {
+      this._trapFocus();
+    }
+  }
 }
diff --git a/src/material/dialog/dialog.ts b/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();
-    }
   }
 
   /**
@@ -468,6 +441,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',

diff --git a/tools/public_api_guard/material/dialog.md b/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;
@@ -162,6 +164,7 @@ export class MatDialogConfig<D = any> {
     closeOnNavigation?: boolean;
     componentFactoryResolver?: ComponentFactoryResolver;
     data?: D | null;
+    delayFocusTrap?: boolean;
     direction?: Direction;
     disableClose?: boolean;
     hasBackdrop?: boolean;
@@ -182,6 +185,8 @@ export class MatDialogConfig<D = any> {
 
 // @public
 export class MatDialogContainer extends _MatDialogContainerBase {
+    // (undocumented)
+    _initializeWithAttachedContent(): void;
     _onAnimationDone({ toState, totalTime }: AnimationEvent_2): void;
     _onAnimationStart({ toState, totalTime }: AnimationEvent_2): void;
     _startExitAnimation(): void;