Skip to content

Commit fd0217d

Browse files
devversionjelbourn
authored andcommitted
fix(dialog): incorrect dialog state for close animation (#19034)
The dialog currently sets its state to `CLOSED` if the close animation is still in progress. This happens due to a misplaced state update assignment. (cherry picked from commit b612fc4)
1 parent 59a5786 commit fd0217d

File tree

2 files changed

+66
-19
lines changed

2 files changed

+66
-19
lines changed

src/material/dialog/dialog-ref.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class MatDialogRef<T, R = any> {
7474
take(1)
7575
).subscribe(() => {
7676
clearTimeout(this._closeFallbackTimeout);
77-
this._overlayRef.dispose();
77+
this._finishDialogClose();
7878
});
7979

8080
_overlayRef.detachments().subscribe(() => {
@@ -119,17 +119,15 @@ export class MatDialogRef<T, R = any> {
119119
.subscribe(event => {
120120
this._beforeClosed.next(dialogResult);
121121
this._beforeClosed.complete();
122-
this._state = MatDialogState.CLOSED;
123122
this._overlayRef.detachBackdrop();
124123

125124
// The logic that disposes of the overlay depends on the exit animation completing, however
126125
// it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback
127126
// timeout which will clean everything up if the animation hasn't fired within the specified
128127
// amount of time plus 100ms. We don't need to run this outside the NgZone, because for the
129128
// vast majority of cases the timeout will have been cleared before it has the chance to fire.
130-
this._closeFallbackTimeout = setTimeout(() => {
131-
this._overlayRef.dispose();
132-
}, event.totalTime + 100);
129+
this._closeFallbackTimeout = setTimeout(() => this._finishDialogClose(),
130+
event.totalTime + 100);
133131
});
134132

135133
this._containerInstance._startExitAnimation();
@@ -223,6 +221,15 @@ export class MatDialogRef<T, R = any> {
223221
return this._state;
224222
}
225223

224+
/**
225+
* Finishes the dialog close by updating the state of the dialog
226+
* and disposing the overlay.
227+
*/
228+
private _finishDialogClose() {
229+
this._state = MatDialogState.CLOSED;
230+
this._overlayRef.dispose();
231+
}
232+
226233
/** Fetches the position strategy object from the overlay ref. */
227234
private _getPositionStrategy(): GlobalPositionStrategy {
228235
return this._overlayRef.getConfig().positionStrategy as GlobalPositionStrategy;

src/material/dialog/dialog.spec.ts

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
ComponentFactoryResolver
2121
} from '@angular/core';
2222
import {By} from '@angular/platform-browser';
23-
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
23+
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
2424
import {Location} from '@angular/common';
2525
import {SpyLocation} from '@angular/common/testing';
2626
import {Directionality} from '@angular/cdk/bidi';
@@ -777,19 +777,6 @@ describe('MatDialog', () => {
777777
expect(resolver.resolveComponentFactory).toHaveBeenCalled();
778778
}));
779779

780-
it('should return the current state of the dialog', fakeAsync(() => {
781-
const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
782-
783-
expect(dialogRef.getState()).toBe(MatDialogState.OPEN);
784-
dialogRef.close();
785-
viewContainerFixture.detectChanges();
786-
787-
expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
788-
flush();
789-
790-
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
791-
}));
792-
793780
describe('passing in data', () => {
794781
it('should be able to pass in data', () => {
795782
let config = {
@@ -1579,6 +1566,59 @@ describe('MatDialog with default options', () => {
15791566
});
15801567

15811568

1569+
describe('MatDialog with animations enabled', () => {
1570+
let dialog: MatDialog;
1571+
let overlayContainer: OverlayContainer;
1572+
1573+
let testViewContainerRef: ViewContainerRef;
1574+
let viewContainerFixture: ComponentFixture<ComponentWithChildViewContainer>;
1575+
1576+
beforeEach(fakeAsync(() => {
1577+
TestBed.configureTestingModule({
1578+
imports: [MatDialogModule, DialogTestModule, BrowserAnimationsModule],
1579+
});
1580+
1581+
TestBed.compileComponents();
1582+
}));
1583+
1584+
beforeEach(inject([MatDialog, OverlayContainer], (d: MatDialog, oc: OverlayContainer) => {
1585+
dialog = d;
1586+
overlayContainer = oc;
1587+
1588+
viewContainerFixture = TestBed.createComponent(ComponentWithChildViewContainer);
1589+
viewContainerFixture.detectChanges();
1590+
testViewContainerRef = viewContainerFixture.componentInstance.childViewContainer;
1591+
}));
1592+
1593+
afterEach(() => {
1594+
overlayContainer.ngOnDestroy();
1595+
});
1596+
1597+
it('should return the current state of the dialog', fakeAsync(() => {
1598+
const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
1599+
// Duration of the close animation in milliseconds. Extracted from the
1600+
// Angular animations definition of the dialog.
1601+
const dialogCloseDuration = 75;
1602+
1603+
expect(dialogRef.getState()).toBe(MatDialogState.OPEN);
1604+
dialogRef.close();
1605+
viewContainerFixture.detectChanges();
1606+
1607+
expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
1608+
1609+
// Ensure that the closing state is still set if half of the animation has
1610+
// passed by. The dialog state should be only set to `closed` when the dialog
1611+
// finished the close animation.
1612+
tick(dialogCloseDuration / 2);
1613+
expect(dialogRef.getState()).toBe(MatDialogState.CLOSING);
1614+
1615+
// Flush the remaining duration of the closing animation. We flush all other remaining
1616+
// tasks (e.g. the fallback close timeout) to avoid fakeAsync pending timer failures.
1617+
flush();
1618+
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
1619+
}));
1620+
});
1621+
15821622
@Directive({selector: 'dir-with-view-container'})
15831623
class DirectiveWithViewContainer {
15841624
constructor(public viewContainerRef: ViewContainerRef) { }

0 commit comments

Comments
 (0)