From cc4fc11071aea71958a974e635441440b7e8c8cc Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 2 Nov 2017 04:27:21 +0100 Subject: [PATCH] fix(dialog): don't block other dialogs from opening while animating (#8051) 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 #6560. --- 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 cca487d49a9..5753f2d5716 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -100,9 +100,6 @@ export class MatDialogContainer extends BasePortalOutlet { /** 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 BasePortalOutlet { // 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 BasePortalOutlet { 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 BasePortalOutlet { } 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 c169de0f8fc..76e22a0e8ae 100644 --- a/src/lib/dialog/dialog-ref.ts +++ b/src/lib/dialog/dialog-ref.ts @@ -161,11 +161,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 8c3047a0146..1dc4cbd9b48 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); this.openDialogs.push(dialogRef); dialogRef.afterClosed().subscribe(() => this._removeOpenDialog(dialogRef)); @@ -253,7 +246,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; }