Skip to content

Commit beea098

Browse files
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 612a738 commit beea098

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

src/material/datepicker/datepicker.spec.ts

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,29 @@ describe('MatDatepicker', () => {
140140
testComponent.opened = false;
141141
fixture.detectChanges();
142142
flush();
143+
fixture.detectChanges();
144+
flush();
145+
fixture.detectChanges();
146+
147+
expect(document.querySelector('.mat-datepicker-content')).toBeNull();
148+
}));
149+
150+
it('should wait for the animation to finish before removing the content', fakeAsync(() => {
151+
testComponent.datepicker.open();
152+
fixture.detectChanges();
153+
flush();
154+
155+
expect(document.querySelector('.mat-datepicker-content')).not.toBeNull();
156+
157+
testComponent.datepicker.close();
158+
fixture.detectChanges();
159+
flush();
160+
161+
expect(document.querySelector('.mat-datepicker-content')).not.toBeNull();
162+
163+
fixture.detectChanges();
164+
flush();
165+
fixture.detectChanges();
143166

144167
expect(document.querySelector('.mat-datepicker-content')).toBeNull();
145168
}));
@@ -178,13 +201,16 @@ describe('MatDatepicker', () => {
178201

179202
const popup = document.querySelector('.cdk-overlay-pane')!;
180203
expect(popup).not.toBeNull();
181-
expect(parseInt(getComputedStyle(popup).height as string)).not.toBe(0);
204+
expect(parseInt(getComputedStyle(popup).height || '0')).not.toBe(0);
182205

183206
testComponent.datepicker.close();
184207
fixture.detectChanges();
185208
flush();
209+
fixture.detectChanges();
210+
flush();
211+
fixture.detectChanges();
186212

187-
expect(parseInt(getComputedStyle(popup).height as string)).toBe(0);
213+
expect(parseInt(getComputedStyle(popup).height || '0')).toBeFalsy();
188214
}));
189215

190216
it('should close the popup when pressing ESCAPE', fakeAsync(() => {
@@ -1092,9 +1118,13 @@ describe('MatDatepicker', () => {
10921118
testComponent.datepicker.close();
10931119
fixture.detectChanges();
10941120
flush();
1121+
fixture.detectChanges();
1122+
flush();
1123+
fixture.detectChanges();
10951124

10961125
testComponent.formField.color = 'warn';
10971126
testComponent.datepicker.open();
1127+
fixture.detectChanges();
10981128

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

src/material/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';
@@ -93,7 +95,8 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
9395
styleUrls: ['datepicker-content.css'],
9496
host: {
9597
'class': 'mat-datepicker-content',
96-
'[@transformPanel]': '"enter"',
98+
'[@transformPanel]': '_animationState',
99+
'(@transformPanel.done)': '_animationDone.next($event)',
97100
'[class.mat-datepicker-content-touch]': 'datepicker.touchUi',
98101
},
99102
animations: [
@@ -106,7 +109,7 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
106109
inputs: ['color'],
107110
})
108111
export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
109-
implements AfterViewInit, CanColor {
112+
implements AfterViewInit, OnDestroy, CanColor {
110113

111114
/** Reference to the internal calendar component. */
112115
@ViewChild(MatCalendar, {static: false}) _calendar: MatCalendar<D>;
@@ -117,13 +120,30 @@ export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
117120
/** Whether the datepicker is above or below the input. */
118121
_isAbove: boolean;
119122

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

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

129149

@@ -359,7 +379,13 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
359379
return;
360380
}
361381
if (this._popupRef && this._popupRef.hasAttached()) {
362-
this._popupRef.detach();
382+
const popupInstance = this._popupComponentRef!.instance;
383+
384+
// We have to wait for the exit animation to finish before detaching the content, because
385+
// we're using a portal outlet to render out the calendar header, which will detach
386+
// immediately in `ngOnDestroy` without waiting for the animation, because the animation
387+
// is on a parent component, which will cause the calendar to jump up.
388+
popupInstance._startExitAnimation().pipe(take(1)).subscribe(() => this._popupRef.detach());
363389
}
364390
if (this._dialogRef) {
365391
this._dialogRef.close();

0 commit comments

Comments
 (0)