From 168883ca19b5cd6a844be87458b8ee0f756eb948 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 26 Oct 2017 20:38:18 +0200 Subject: [PATCH] fix(dialog): don't block other dialogs from opening while animating The behavior where we don't allow new dialogs to be opened while a dialog is animating was introduced initially in #5769 to avoid issues where the user might open multiple dialogs by accident if they press enter one too many times. This is problematic because it also means that the consumer is blocked from opening dialogs for certain 300ms intervals. These changes approach the issue differently by moving focus onto the dialog container immediately while it's animating. Fixes #5769. --- src/lib/dialog/dialog-container.ts | 18 ++++++------------ src/lib/dialog/dialog-ref.ts | 5 ----- src/lib/dialog/dialog.ts | 11 ++--------- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index 91d0c9ba96db..372f3d205295 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -100,9 +100,6 @@ export class MatDialogContainer extends BasePortalHost { /** ID of the element that should be considered as the dialog's label. */ _ariaLabelledBy: string | null = null; - /** Whether the container is currently mid-animation. */ - _isAnimating = false; - constructor( private _elementRef: ElementRef, private _focusTrapFactory: FocusTrapFactory, @@ -147,13 +144,7 @@ export class MatDialogContainer extends BasePortalHost { // If were to attempt to focus immediately, then the content of the dialog would not yet be // ready in instances where change detection has to run first. To deal with this, we simply // wait for the microtask queue to be empty. - this._focusTrap.focusInitialElementWhenReady().then(hasMovedFocus => { - // If we didn't find any focusable elements inside the dialog, focus the - // container so the user can't tab into other elements behind it. - if (!hasMovedFocus) { - this._elementRef.nativeElement.focus(); - } - }); + this._focusTrap.focusInitialElementWhenReady(); } /** Restores focus to the element that was focused before the dialog opened. */ @@ -174,6 +165,11 @@ export class MatDialogContainer extends BasePortalHost { private _savePreviouslyFocusedElement() { if (this._document) { this._elementFocusedBeforeDialogWasOpened = this._document.activeElement as HTMLElement; + + // Move focus onto the dialog immediately in order to prevent the user from accidentally + // opening multiple dialogs at the same time. Needs to be async, because the element + // may not be focusable immediately. + Promise.resolve().then(() => this._elementRef.nativeElement.focus()); } } @@ -186,12 +182,10 @@ export class MatDialogContainer extends BasePortalHost { } this._animationStateChanged.emit(event); - this._isAnimating = false; } /** Callback, invoked when an animation on the host starts. */ _onAnimationStart(event: AnimationEvent) { - this._isAnimating = true; this._animationStateChanged.emit(event); } diff --git a/src/lib/dialog/dialog-ref.ts b/src/lib/dialog/dialog-ref.ts index 8c8f29480eb1..6b197a47daf4 100644 --- a/src/lib/dialog/dialog-ref.ts +++ b/src/lib/dialog/dialog-ref.ts @@ -150,11 +150,6 @@ export class MatDialogRef { return this; } - /** Returns whether the dialog is animating. */ - _isAnimating(): boolean { - return this._containerInstance._isAnimating; - } - /** Fetches the position strategy object from the overlay ref. */ private _getPositionStrategy(): GlobalPositionStrategy { return this._overlayRef.getConfig().positionStrategy as GlobalPositionStrategy; diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index ce0b39d71c3f..72def75c26e7 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -117,13 +117,6 @@ export class MatDialog { open(componentOrTemplateRef: ComponentType | TemplateRef, config?: MatDialogConfig): MatDialogRef { - const inProgressDialog = this.openDialogs.find(dialog => dialog._isAnimating()); - - // If there's a dialog that is in the process of being opened, return it instead. - if (inProgressDialog) { - return inProgressDialog; - } - config = _applyConfigDefaults(config); if (config.id && this.getDialogById(config.id)) { @@ -133,7 +126,7 @@ export class MatDialog { const overlayRef = this._createOverlay(config); const dialogContainer = this._attachDialogContainer(overlayRef, config); const dialogRef = - this._attachDialogContent(componentOrTemplateRef, dialogContainer, overlayRef, config); + this._attachDialogContent(componentOrTemplateRef, dialogContainer, overlayRef, config); if (!this.openDialogs.length) { document.addEventListener('keydown', this._boundKeydown); @@ -252,7 +245,7 @@ export class MatDialog { { $implicit: config.data, dialogRef })); } else { const injector = this._createInjector(config, dialogRef, dialogContainer); - const contentRef = dialogContainer.attachComponentPortal( + const contentRef = dialogContainer.attachComponentPortal( new ComponentPortal(componentOrTemplateRef, undefined, injector)); dialogRef.componentInstance = contentRef.instance; }