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
The behavior where we don't allow new dialogs to be opened while a dialog is animating was introduced initially in angular#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 angular#5769.
  • Loading branch information
crisbeto committed Oct 26, 2017
1 parent 0ea4370 commit 168883c
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 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,
Expand Down Expand Up @@ -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. */
Expand All @@ -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());
}
}

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

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 @@ -150,11 +150,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);

if (!this.openDialogs.length) {
document.addEventListener('keydown', this._boundKeydown);
Expand Down Expand Up @@ -252,7 +245,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 168883c

Please sign in to comment.