From bace7f9bacd6b9838675d44cddcffb45a68ac8f9 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 30 May 2019 08:29:54 +0200 Subject: [PATCH] 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. --- src/material/datepicker/datepicker.spec.ts | 34 +++++++++++++++++-- src/material/datepicker/datepicker.ts | 34 ++++++++++++++++--- .../public_api_guard/material/datepicker.d.ts | 6 +++- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/material/datepicker/datepicker.spec.ts b/src/material/datepicker/datepicker.spec.ts index ab5bcedc18c8..63b62929e959 100644 --- a/src/material/datepicker/datepicker.spec.ts +++ b/src/material/datepicker/datepicker.spec.ts @@ -140,6 +140,29 @@ describe('MatDatepicker', () => { testComponent.opened = false; fixture.detectChanges(); flush(); + fixture.detectChanges(); + flush(); + fixture.detectChanges(); + + expect(document.querySelector('.mat-datepicker-content')).toBeNull(); + })); + + it('should wait for the animation to finish before removing the content', fakeAsync(() => { + testComponent.datepicker.open(); + fixture.detectChanges(); + flush(); + + expect(document.querySelector('.mat-datepicker-content')).not.toBeNull(); + + testComponent.datepicker.close(); + fixture.detectChanges(); + flush(); + + expect(document.querySelector('.mat-datepicker-content')).not.toBeNull(); + + fixture.detectChanges(); + flush(); + fixture.detectChanges(); expect(document.querySelector('.mat-datepicker-content')).toBeNull(); })); @@ -178,13 +201,16 @@ describe('MatDatepicker', () => { const popup = document.querySelector('.cdk-overlay-pane')!; expect(popup).not.toBeNull(); - expect(parseInt(getComputedStyle(popup).height as string)).not.toBe(0); + expect(parseInt(getComputedStyle(popup).height || '0')).not.toBe(0); testComponent.datepicker.close(); fixture.detectChanges(); flush(); + fixture.detectChanges(); + flush(); + fixture.detectChanges(); - expect(parseInt(getComputedStyle(popup).height as string)).toBe(0); + expect(parseInt(getComputedStyle(popup).height || '0')).toBeFalsy(); })); it('should close the popup when pressing ESCAPE', fakeAsync(() => { @@ -1092,9 +1118,13 @@ describe('MatDatepicker', () => { testComponent.datepicker.close(); fixture.detectChanges(); flush(); + fixture.detectChanges(); + flush(); + fixture.detectChanges(); testComponent.formField.color = 'warn'; testComponent.datepicker.open(); + fixture.detectChanges(); contentEl = document.querySelector('.mat-datepicker-content')!; fixture.detectChanges(); diff --git a/src/material/datepicker/datepicker.ts b/src/material/datepicker/datepicker.ts index 8a5dc4faf70f..60181c1cb827 100644 --- a/src/material/datepicker/datepicker.ts +++ b/src/material/datepicker/datepicker.ts @@ -21,6 +21,7 @@ import {DOCUMENT} from '@angular/common'; import { AfterViewInit, ChangeDetectionStrategy, + ChangeDetectorRef, Component, ComponentRef, ElementRef, @@ -44,6 +45,7 @@ import { ThemePalette, } from '@angular/material/core'; import {MatDialog, MatDialogRef} from '@angular/material/dialog'; +import {AnimationEvent} from '@angular/animations'; import {merge, Subject, Subscription} from 'rxjs'; import {filter, take} from 'rxjs/operators'; import {MatCalendar} from './calendar'; @@ -93,7 +95,8 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent styleUrls: ['datepicker-content.css'], host: { 'class': 'mat-datepicker-content', - '[@transformPanel]': '"enter"', + '[@transformPanel]': '_animationState', + '(@transformPanel.done)': '_animationDone.next($event)', '[class.mat-datepicker-content-touch]': 'datepicker.touchUi', }, animations: [ @@ -106,7 +109,7 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent inputs: ['color'], }) export class MatDatepickerContent extends _MatDatepickerContentMixinBase - implements AfterViewInit, CanColor { + implements AfterViewInit, OnDestroy, CanColor { /** Reference to the internal calendar component. */ @ViewChild(MatCalendar, {static: false}) _calendar: MatCalendar; @@ -117,13 +120,30 @@ export class MatDatepickerContent extends _MatDatepickerContentMixinBase /** Whether the datepicker is above or below the input. */ _isAbove: boolean; - constructor(elementRef: ElementRef) { + /** State of the datepicker's animation. */ + _animationState: 'enter' | 'void' = 'enter'; + + /** Emits whenever an animation on the datepicker completes. */ + _animationDone = new Subject(); + + constructor(elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef) { super(elementRef); } + /** Starts the datepicker's exiting animation. */ + _startExitAnimation() { + this._animationState = 'void'; + this._changeDetectorRef.markForCheck(); + return this._animationDone; + } + ngAfterViewInit() { this._calendar.focusActiveCell(); } + + ngOnDestroy() { + this._animationDone.complete(); + } } @@ -359,7 +379,13 @@ export class MatDatepicker implements OnDestroy, CanColor { return; } if (this._popupRef && this._popupRef.hasAttached()) { - this._popupRef.detach(); + const popupInstance = this._popupComponentRef!.instance; + + // We have to wait for the exit animation to finish before detaching the content, because + // we're using a portal outlet to render out the calendar header, which will detach + // immediately in `ngOnDestroy` without waiting for the animation, because the animation + // is on a parent component, which will cause the calendar to jump up. + popupInstance._startExitAnimation().pipe(take(1)).subscribe(() => this._popupRef.detach()); } if (this._dialogRef) { this._dialogRef.close(); diff --git a/tools/public_api_guard/material/datepicker.d.ts b/tools/public_api_guard/material/datepicker.d.ts index 4169926a92a5..ba05219f3f77 100644 --- a/tools/public_api_guard/material/datepicker.d.ts +++ b/tools/public_api_guard/material/datepicker.d.ts @@ -139,11 +139,15 @@ export declare const matDatepickerAnimations: { }; export declare class MatDatepickerContent extends _MatDatepickerContentMixinBase implements AfterViewInit, CanColor { + _animationDone: Subject; + _animationState: 'enter' | 'void'; _calendar: MatCalendar; _isAbove: boolean; datepicker: MatDatepicker; - constructor(elementRef: ElementRef); + constructor(elementRef: ElementRef, _changeDetectorRef: ChangeDetectorRef); + _startExitAnimation(): Subject; ngAfterViewInit(): void; + ngOnDestroy(): void; } export declare class MatDatepickerInput implements ControlValueAccessor, OnDestroy, Validator {