Skip to content

Commit

Permalink
fix(dialog): don't block other dialogs from opening while animating (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
crisbeto authored and andrewseguin committed Nov 2, 2017
1 parent d17f9d2 commit cc4fc11
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 26 deletions.
18 changes: 6 additions & 12 deletions src/lib/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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. */
Expand All @@ -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());
}
}

Expand All @@ -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);
}

Expand Down
5 changes: 0 additions & 5 deletions src/lib/dialog/dialog-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ export class MatDialogRef<T> {
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;
Expand Down
11 changes: 2 additions & 9 deletions src/lib/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,6 @@ export class MatDialog {
open<T, D = any>(componentOrTemplateRef: ComponentType<T> | TemplateRef<T>,
config?: MatDialogConfig<D>): MatDialogRef<T> {

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)) {
Expand All @@ -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<T>(componentOrTemplateRef, dialogContainer, overlayRef, config);

this.openDialogs.push(dialogRef);
dialogRef.afterClosed().subscribe(() => this._removeOpenDialog(dialogRef));
Expand Down Expand Up @@ -253,7 +246,7 @@ export class MatDialog {
<any>{ $implicit: config.data, dialogRef }));
} else {
const injector = this._createInjector<T>(config, dialogRef, dialogContainer);
const contentRef = dialogContainer.attachComponentPortal(
const contentRef = dialogContainer.attachComponentPortal<T>(
new ComponentPortal(componentOrTemplateRef, undefined, injector));
dialogRef.componentInstance = contentRef.instance;
}
Expand Down

0 comments on commit cc4fc11

Please sign in to comment.