Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animations): ensure position and display styles are handled outside of keyframes/web-animations #28911

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -10,6 +10,7 @@ import {AnimationPlayer, ɵStyleData} from '@angular/animations';
import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle} from '../../util';
import {AnimationDriver} from '../animation_driver';
import {containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles';

import {CssKeyframesPlayer} from './css_keyframes_player';
import {DirectStylePlayer} from './direct_style_player';
Expand Down Expand Up @@ -105,8 +106,9 @@ export class CssKeyframesDriver implements AnimationDriver {
const kfElm = this.buildKeyframeElement(element, animationName, keyframes);
document.querySelector('head') !.appendChild(kfElm);

const specialStyles = packageNonAnimatableStyles(element, keyframes);
const player = new CssKeyframesPlayer(
element, keyframes, animationName, duration, delay, easing, finalStyles);
element, keyframes, animationName, duration, delay, easing, finalStyles, specialStyles);

player.onDestroy(() => removeElement(kfElm));
return player;
Expand Down
Expand Up @@ -8,12 +8,11 @@
import {AnimationPlayer} from '@angular/animations';

import {computeStyle} from '../../util';

import {SpecialCasedStyles} from '../special_cased_styles';
import {ElementAnimationStyleHandler} from './element_animation_style_handler';

const DEFAULT_FILL_MODE = 'forwards';
const DEFAULT_EASING = 'linear';
const ANIMATION_END_EVENT = 'animationend';

export const enum AnimatorControlState {INITIALIZED = 1, STARTED = 2, FINISHED = 3, DESTROYED = 4}

Expand All @@ -38,7 +37,8 @@ export class CssKeyframesPlayer implements AnimationPlayer {
public readonly element: any, public readonly keyframes: {[key: string]: string | number}[],
public readonly animationName: string, private readonly _duration: number,
private readonly _delay: number, easing: string,
private readonly _finalStyles: {[key: string]: any}) {
private readonly _finalStyles: {[key: string]: any},
private readonly _specialStyles?: SpecialCasedStyles|null) {
this.easing = easing || DEFAULT_EASING;
this.totalTime = _duration + _delay;
this._buildStyler();
Expand All @@ -57,6 +57,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
this._styler.destroy();
this._flushStartFns();
this._flushDoneFns();
if (this._specialStyles) {
this._specialStyles.destroy();
}
this._onDestroyFns.forEach(fn => fn());
this._onDestroyFns = [];
}
Expand All @@ -77,6 +80,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
this._state = AnimatorControlState.FINISHED;
this._styler.finish();
this._flushStartFns();
if (this._specialStyles) {
this._specialStyles.finish();
}
this._flushDoneFns();
}

Expand All @@ -100,6 +106,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
if (!this.hasStarted()) {
this._flushStartFns();
this._state = AnimatorControlState.STARTED;
if (this._specialStyles) {
this._specialStyles.start();
}
}
this._styler.resume();
}
Expand Down
133 changes: 133 additions & 0 deletions packages/animations/browser/src/render/special_cased_styles.ts
@@ -0,0 +1,133 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {eraseStyles, setStyles} from '../util';

/**
* Returns an instance of `SpecialCasedStyles` if and when any special (non animateable) styles are
* detected.
*
* In CSS there exist properties that cannot be animated within a keyframe animation
* (whether it be via CSS keyframes or web-animations) and the animation implementation
* will ignore them. This function is designed to detect those special cased styles and
* return a container that will be executed at the start and end of the animation.
*
* @returns an instance of `SpecialCasedStyles` if any special styles are detected otherwise `null`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the docs.

*/
export function packageNonAnimatableStyles(
element: any, styles: {[key: string]: any} | {[key: string]: any}[]): SpecialCasedStyles|null {
let startStyles: {[key: string]: any}|null = null;
let endStyles: {[key: string]: any}|null = null;
if (Array.isArray(styles) && styles.length) {
startStyles = filterNonAnimatableStyles(styles[0]);
if (styles.length > 1) {
endStyles = filterNonAnimatableStyles(styles[styles.length - 1]);
}
} else if (styles) {
startStyles = filterNonAnimatableStyles(styles);
}

return (startStyles || endStyles) ? new SpecialCasedStyles(element, startStyles, endStyles) :
null;
}

/**
* Designed to be executed during a keyframe-based animation to apply any special-cased styles.
*
* When started (when the `start()` method is run) then the provided `startStyles`
* will be applied. When finished (when the `finish()` method is called) the
* `endStyles` will be applied as well any any starting styles. Finally when
* `destroy()` is called then all styles will be removed.
*/
export class SpecialCasedStyles {
static initialStylesByElement = new WeakMap<any, {[key: string]: any}>();

private _state = SpecialCasedStylesState.Pending;
private _initialStyles !: {[key: string]: any};

constructor(
private _element: any, private _startStyles: {[key: string]: any}|null,
private _endStyles: {[key: string]: any}|null) {
let initialStyles = SpecialCasedStyles.initialStylesByElement.get(_element);
if (!initialStyles) {
SpecialCasedStyles.initialStylesByElement.set(_element, initialStyles = {});
}
this._initialStyles = initialStyles;
}

start() {
if (this._state < SpecialCasedStylesState.Started) {
if (this._startStyles) {
setStyles(this._element, this._startStyles, this._initialStyles);
}
this._state = SpecialCasedStylesState.Started;
}
}

finish() {
this.start();
if (this._state < SpecialCasedStylesState.Finished) {
setStyles(this._element, this._initialStyles);
if (this._endStyles) {
setStyles(this._element, this._endStyles);
this._endStyles = null;
}
this._state = SpecialCasedStylesState.Started;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matsko Should line 79 be

this._state = SpecialCasedStylesState.Finished; ?

I think the order of the SpecialCasedStylesState enum allowed this to still work. Please excuse me if this line was intentional for other purposes!

}
}

destroy() {
this.finish();
if (this._state < SpecialCasedStylesState.Destroyed) {
SpecialCasedStyles.initialStylesByElement.delete(this._element);
if (this._startStyles) {
eraseStyles(this._element, this._startStyles);
this._endStyles = null;
}
if (this._endStyles) {
eraseStyles(this._element, this._endStyles);
this._endStyles = null;
}
setStyles(this._element, this._initialStyles);
this._state = SpecialCasedStylesState.Destroyed;
}
}
}

/**
* An enum of states reflective of what the status of `SpecialCasedStyles` is.
*
* Depending on how `SpecialCasedStyles` is interacted with, the start and end
* styles may not be applied in the same way. This enum ensures that if and when
* the ending styles are applied then the starting styles are applied. It is
* also used to reflect what the current status of the special cased styles are
* which helps prevent the starting/ending styles not be applied twice. It is
* also used to cleanup the styles once `SpecialCasedStyles` is destroyed.
*/
const enum SpecialCasedStylesState {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could use some docs.

Pending = 0,
Started = 1,
Finished = 2,
Destroyed = 3,
}

function filterNonAnimatableStyles(styles: {[key: string]: any}) {
let result: {[key: string]: any}|null = null;
const props = Object.keys(styles);
for (let i = 0; i < props.length; i++) {
const prop = props[i];
if (isNonAnimatableStyle(prop)) {
result = result || {};
result[prop] = styles[prop];
}
}
return result;
}

function isNonAnimatableStyle(prop: string) {
return prop === 'display' || prop === 'position';
}
Expand Up @@ -11,6 +11,7 @@ import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copy
import {AnimationDriver} from '../animation_driver';
import {CssKeyframesDriver} from '../css_keyframes/css_keyframes_driver';
import {containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared';
import {packageNonAnimatableStyles} from '../special_cased_styles';

import {WebAnimationsPlayer} from './web_animations_player';

Expand Down Expand Up @@ -66,7 +67,8 @@ export class WebAnimationsDriver implements AnimationDriver {

keyframes = keyframes.map(styles => copyStyles(styles, false));
keyframes = balancePreviousStylesIntoKeyframes(element, keyframes, previousStyles);
return new WebAnimationsPlayer(element, keyframes, playerOptions);
const specialStyles = packageNonAnimatableStyles(element, keyframes);
return new WebAnimationsPlayer(element, keyframes, playerOptions, specialStyles);
}
}

Expand Down
Expand Up @@ -7,7 +7,8 @@
*/
import {AnimationPlayer} from '@angular/animations';

import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle, copyStyles} from '../../util';
import {computeStyle} from '../../util';
import {SpecialCasedStyles} from '../special_cased_styles';

import {DOMAnimation} from './dom_animation';

Expand All @@ -33,7 +34,8 @@ export class WebAnimationsPlayer implements AnimationPlayer {

constructor(
public element: any, public keyframes: {[key: string]: string | number}[],
public options: {[key: string]: string | number}) {
public options: {[key: string]: string | number},
private _specialStyles?: SpecialCasedStyles|null) {
this._duration = <number>options['duration'];
this._delay = <number>options['delay'] || 0;
this.time = this._duration + this._delay;
Expand Down Expand Up @@ -91,6 +93,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
this._onStartFns.forEach(fn => fn());
this._onStartFns = [];
this._started = true;
if (this._specialStyles) {
this._specialStyles.start();
}
}
this.domPlayer.play();
}
Expand All @@ -102,6 +107,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {

finish(): void {
this.init();
if (this._specialStyles) {
this._specialStyles.finish();
}
this._onFinish();
this.domPlayer.finish();
}
Expand Down Expand Up @@ -131,6 +139,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
this._destroyed = true;
this._resetDomPlayerState();
this._onFinish();
if (this._specialStyles) {
this._specialStyles.destroy();
}
this._onDestroyFns.forEach(fn => fn());
this._onDestroyFns = [];
}
Expand Down
5 changes: 4 additions & 1 deletion packages/animations/browser/src/util.ts
Expand Up @@ -157,10 +157,13 @@ function writeStyleAttribute(element: any) {
element.setAttribute('style', styleAttrValue);
}

export function setStyles(element: any, styles: ɵStyleData) {
export function setStyles(element: any, styles: ɵStyleData, formerStyles?: {[key: string]: any}) {
if (element['style']) {
Object.keys(styles).forEach(prop => {
const camelProp = dashCaseToCamelCase(prop);
if (formerStyles && !formerStyles.hasOwnProperty(prop)) {
formerStyles[prop] = element.style[camelProp];
}
element.style[camelProp] = styles[prop];
});
// On the server set the 'style' attribute since it's not automatically reflected.
Expand Down
Expand Up @@ -308,13 +308,65 @@ import {TestBed} from '../../testing';

expect(foo.style.getPropertyValue('max-height')).toBeFalsy();
});

it('should apply the `display` and `position` styles as regular inline styles for the duration of the animation',
() => {
@Component({
selector: 'ani-cmp',
template: `
<div #elm [@myAnimation]="myAnimationExp" style="display:table; position:fixed"></div>
`,
animations: [
trigger(
'myAnimation',
[
state('go', style({display: 'inline-block'})),
transition(
'* => go',
[
style({display: 'inline', position: 'absolute', opacity: 0}),
animate('1s', style({display: 'inline', opacity: 1, position: 'static'})),
animate('1s', style({display: 'flexbox', opacity: 0})),
])
]),
]
})
class Cmp {
@ViewChild('elm') public element: any;

public myAnimationExp = '';
}

TestBed.configureTestingModule({declarations: [Cmp]});

const engine = TestBed.get(AnimationEngine);
const fixture = TestBed.createComponent(Cmp);
const cmp = fixture.componentInstance;

// In Ivy, change detection needs to run before the ViewQuery for cmp.element will resolve.
// Keeping this test enabled since we still want to test the animation logic in Ivy.
if (ivyEnabled) fixture.detectChanges();

const elm = cmp.element.nativeElement;
expect(elm.style.getPropertyValue('display')).toEqual('table');
expect(elm.style.getPropertyValue('position')).toEqual('fixed');

cmp.myAnimationExp = 'go';
fixture.detectChanges();

expect(elm.style.getPropertyValue('display')).toEqual('inline');
expect(elm.style.getPropertyValue('position')).toEqual('absolute');

const player = engine.players.pop();
player.finish();
player.destroy();

expect(elm.style.getPropertyValue('display')).toEqual('inline-block');
expect(elm.style.getPropertyValue('position')).toEqual('fixed');
});
});
})();

function approximate(value: number, target: number) {
return Math.abs(target - value) / value;
}

function getPlayer(engine: AnimationEngine, index = 0) {
return (engine.players[index] as any) !.getRealPlayer();
}
Expand Down