Skip to content

Commit 4cbef16

Browse files
crisbetoVivian Hu
authored and
Vivian Hu
committed
fix(datepicker): wait for exit animation to finish before detaching content
This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since #9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.
1 parent 6c741c4 commit 4cbef16

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

src/lib/datepicker/datepicker.spec.ts

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,29 @@ describe('MatDatepicker', () => {
148148
testComponent.opened = false;
149149
fixture.detectChanges();
150150
flush();
151+
fixture.detectChanges();
152+
flush();
153+
fixture.detectChanges();
154+
155+
expect(document.querySelector('.mat-datepicker-content')).toBeNull();
156+
}));
157+
158+
it('should wait for the animation to finish before removing the content', fakeAsync(() => {
159+
testComponent.datepicker.open();
160+
fixture.detectChanges();
161+
flush();
162+
163+
expect(document.querySelector('.mat-datepicker-content')).not.toBeNull();
164+
165+
testComponent.datepicker.close();
166+
fixture.detectChanges();
167+
flush();
168+
169+
expect(document.querySelector('.mat-datepicker-content')).not.toBeNull();
170+
171+
fixture.detectChanges();
172+
flush();
173+
fixture.detectChanges();
151174

152175
expect(document.querySelector('.mat-datepicker-content')).toBeNull();
153176
}));
@@ -186,13 +209,16 @@ describe('MatDatepicker', () => {
186209

187210
const popup = document.querySelector('.cdk-overlay-pane')!;
188211
expect(popup).not.toBeNull();
189-
expect(parseInt(getComputedStyle(popup).height as string)).not.toBe(0);
212+
expect(parseInt(getComputedStyle(popup).height || '0')).not.toBe(0);
190213

191214
testComponent.datepicker.close();
192215
fixture.detectChanges();
193216
flush();
217+
fixture.detectChanges();
218+
flush();
219+
fixture.detectChanges();
194220

195-
expect(parseInt(getComputedStyle(popup).height as string)).toBe(0);
221+
expect(parseInt(getComputedStyle(popup).height || '0')).toBe(0);
196222
}));
197223

198224
it('should close the popup when pressing ESCAPE', fakeAsync(() => {
@@ -1085,9 +1111,13 @@ describe('MatDatepicker', () => {
10851111
testComponent.datepicker.close();
10861112
fixture.detectChanges();
10871113
flush();
1114+
fixture.detectChanges();
1115+
flush();
1116+
fixture.detectChanges();
10881117

10891118
testComponent.formField.color = 'warn';
10901119
testComponent.datepicker.open();
1120+
fixture.detectChanges();
10911121

10921122
contentEl = document.querySelector('.mat-datepicker-content')!;
10931123
fixture.detectChanges();

src/lib/datepicker/datepicker.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {DOCUMENT} from '@angular/common';
2121
import {
2222
AfterViewInit,
2323
ChangeDetectionStrategy,
24+
ChangeDetectorRef,
2425
Component,
2526
ComponentRef,
2627
ElementRef,
@@ -44,6 +45,7 @@ import {
4445
ThemePalette,
4546
} from '@angular/material/core';
4647
import {MatDialog, MatDialogRef} from '@angular/material/dialog';
48+
import {AnimationEvent} from '@angular/animations';
4749
import {merge, Subject, Subscription} from 'rxjs';
4850
import {filter, take} from 'rxjs/operators';
4951
import {MatCalendar} from './calendar';
@@ -92,7 +94,8 @@ export const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepicker
9294
styleUrls: ['datepicker-content.css'],
9395
host: {
9496
'class': 'mat-datepicker-content',
95-
'[@transformPanel]': '"enter"',
97+
'[@transformPanel]': '_animationState',
98+
'(@transformPanel.done)': '_animationDone.next($event)',
9699
'[class.mat-datepicker-content-touch]': 'datepicker.touchUi',
97100
},
98101
animations: [
@@ -105,7 +108,7 @@ export const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepicker
105108
inputs: ['color'],
106109
})
107110
export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
108-
implements AfterViewInit, CanColor {
111+
implements AfterViewInit, OnDestroy, CanColor {
109112

110113
/** Reference to the internal calendar component. */
111114
@ViewChild(MatCalendar) _calendar: MatCalendar<D>;
@@ -116,13 +119,30 @@ export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
116119
/** Whether the datepicker is above or below the input. */
117120
_isAbove: boolean;
118121

119-
constructor(elementRef: ElementRef) {
122+
/** State of the datepicker's animation. */
123+
_animationState: 'enter' | 'void' = 'enter';
124+
125+
/** Emits whenever an animation on the datepicker completes. */
126+
_animationDone = new Subject<AnimationEvent>();
127+
128+
constructor(elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef) {
120129
super(elementRef);
121130
}
122131

132+
/** Starts the datepicker's exiting animation. */
133+
_startExitAnimation() {
134+
this._animationState = 'void';
135+
this._changeDetectorRef.markForCheck();
136+
return this._animationDone;
137+
}
138+
123139
ngAfterViewInit() {
124140
this._calendar.focusActiveCell();
125141
}
142+
143+
ngOnDestroy() {
144+
this._animationDone.complete();
145+
}
126146
}
127147

128148

@@ -355,7 +375,13 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
355375
return;
356376
}
357377
if (this._popupRef && this._popupRef.hasAttached()) {
358-
this._popupRef.detach();
378+
const popupInstance = this._popupComponentRef!.instance;
379+
380+
// We have to wait for the exit animation to finish before detaching the content, because
381+
// we're using a portal outlet to render out the calendar header, which will detach
382+
// immediately in `ngOnDestroy` without waiting for the animation, because the animation
383+
// is on a parent component, which will cause the calendar to jump up.
384+
popupInstance._startExitAnimation().pipe(take(1)).subscribe(() => this._popupRef.detach());
359385
}
360386
if (this._dialogRef) {
361387
this._dialogRef.close();

0 commit comments

Comments
 (0)