Skip to content

Commit

Permalink
fix(material-experimental/mdc-dialog): afterClosed being run outside …
Browse files Browse the repository at this point in the history
…of NgZone (angular#21702)

Fixes that all of the callbacks tied to animations within the MDC-based dialog were being run
outside of the `NgZone`.

Fixes angular#21696.
  • Loading branch information
crisbeto authored and wagnermaciel committed Feb 8, 2021
1 parent 43147cc commit 06343ab
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/material-experimental/mdc-dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
Component,
ElementRef,
Inject,
NgZone,
OnDestroy,
Optional,
ViewEncapsulation
Expand Down Expand Up @@ -66,7 +65,6 @@ export class MatDialogContainer extends _MatDialogContainerBase implements OnDes
changeDetectorRef: ChangeDetectorRef,
@Optional() @Inject(DOCUMENT) document: any,
config: MatDialogConfig,
private _ngZone: NgZone,
@Optional() @Inject(ANIMATION_MODULE_TYPE) private _animationMode?: string,
focusMonitor?: FocusMonitor) {
super(elementRef, focusTrapFactory, changeDetectorRef, document, config, focusMonitor);
Expand Down Expand Up @@ -180,6 +178,9 @@ export class MatDialogContainer extends _MatDialogContainerBase implements OnDes
if (this._animationTimer !== null) {
clearTimeout(this._animationTimer);
}
this._ngZone.runOutsideAngular(() => this._animationTimer = setTimeout(callback, duration));

// Note that we want this timer to run inside the NgZone, because we want
// the related events like `afterClosed` to be inside the zone as well.
this._animationTimer = setTimeout(callback, duration);
}
}
18 changes: 18 additions & 0 deletions src/material-experimental/mdc-dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
Inject,
Injector,
NgModule,
NgZone,
TemplateRef,
ViewChild,
ViewContainerRef
Expand Down Expand Up @@ -201,6 +202,23 @@ describe('MDC-based MatDialog', () => {
expect(overlayContainerElement.querySelector('mat-mdc-dialog-container')).toBeNull();
}));

it('should invoke the afterClosed callback inside the NgZone',
fakeAsync(inject([NgZone], (zone: NgZone) => {
const dialogRef = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
const afterCloseCallback = jasmine.createSpy('afterClose callback');

dialogRef.afterClosed().subscribe(() => {
afterCloseCallback(NgZone.isInAngularZone());
});
zone.run(() => {
dialogRef.close();
viewContainerFixture.detectChanges();
flush();
});

expect(afterCloseCallback).toHaveBeenCalledWith(true);
})));

it('should dispose of dialog if view container is destroyed while animating', fakeAsync(() => {
const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});

Expand Down
20 changes: 19 additions & 1 deletion src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
TemplateRef,
ViewChild,
ViewContainerRef,
ComponentFactoryResolver
ComponentFactoryResolver,
NgZone
} from '@angular/core';
import {By} from '@angular/platform-browser';
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -204,6 +205,23 @@ describe('MatDialog', () => {
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
}));

it('should invoke the afterClosed callback inside the NgZone',
fakeAsync(inject([NgZone], (zone: NgZone) => {
const dialogRef = dialog.open(PizzaMsg, { viewContainerRef: testViewContainerRef });
const afterCloseCallback = jasmine.createSpy('afterClose callback');

dialogRef.afterClosed().subscribe(() => {
afterCloseCallback(NgZone.isInAngularZone());
});
zone.run(() => {
dialogRef.close();
viewContainerFixture.detectChanges();
flush();
});

expect(afterCloseCallback).toHaveBeenCalledWith(true);
})));

it('should dispose of dialog if view container is destroyed while animating', fakeAsync(() => {
const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});

Expand Down

0 comments on commit 06343ab

Please sign in to comment.