From da9ff255ddadda86ccb78ae4d638cd05ea110a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 1 May 2018 10:40:11 -0700 Subject: [PATCH] fix(animations): properly clean up queried element styles in safari/edge (#23633) Prior to this patch, if an element is queried and animated for 0 seconds (just a style() call and nothing else) then the styles applied would not be properly cleaned up due to their camelCased nature. PR Close #23633 --- .../css_keyframes/css_keyframes_driver.ts | 11 +--- .../css_keyframes/direct_style_player.ts | 12 +++- .../animations/browser/src/render/shared.ts | 9 +++ ...s_keyframes_animations_integration_spec.ts | 55 +++++++++++++++++++ 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts b/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts index 384b438df4..242a1fd1e7 100644 --- a/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts +++ b/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts @@ -9,7 +9,7 @@ import {AnimationPlayer, ɵStyleData} from '@angular/animations'; import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle} from '../../util'; import {AnimationDriver} from '../animation_driver'; -import {containsElement, invokeQuery, matchesElement, validateStyleProperty} from '../shared'; +import {containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared'; import {CssKeyframesPlayer} from './css_keyframes_player'; import {DirectStylePlayer} from './direct_style_player'; @@ -137,15 +137,6 @@ function flattenKeyframesIntoStyles( return flatKeyframes; } -function hypenatePropsObject(object: {[key: string]: any}): {[key: string]: any} { - const newObj: {[key: string]: any} = {}; - Object.keys(object).forEach(prop => { - const newProp = prop.replace(/([a-z])([A-Z])/g, '$1-$2'); - newObj[newProp] = object[prop]; - }); - return newObj; -} - function removeElement(node: any) { node.parentNode.removeChild(node); } diff --git a/packages/animations/browser/src/render/css_keyframes/direct_style_player.ts b/packages/animations/browser/src/render/css_keyframes/direct_style_player.ts index e397d5a677..f4c092d27e 100644 --- a/packages/animations/browser/src/render/css_keyframes/direct_style_player.ts +++ b/packages/animations/browser/src/render/css_keyframes/direct_style_player.ts @@ -6,12 +6,17 @@ * found in the LICENSE file at https://angular.io/license */ import {NoopAnimationPlayer} from '@angular/animations'; +import {hypenatePropsObject} from '../shared'; export class DirectStylePlayer extends NoopAnimationPlayer { private _startingStyles: {[key: string]: any}|null = {}; private __initialized = false; + private _styles: {[key: string]: any}; - constructor(public element: any, private _styles: {[key: string]: any}) { super(); } + constructor(public element: any, styles: {[key: string]: any}) { + super(); + this._styles = hypenatePropsObject(styles); + } init() { if (this.__initialized || !this._startingStyles) return; @@ -25,7 +30,8 @@ export class DirectStylePlayer extends NoopAnimationPlayer { play() { if (!this._startingStyles) return; this.init(); - Object.keys(this._styles).forEach(prop => { this.element.style[prop] = this._styles[prop]; }); + Object.keys(this._styles) + .forEach(prop => this.element.style.setProperty(prop, this._styles[prop])); super.play(); } @@ -34,7 +40,7 @@ export class DirectStylePlayer extends NoopAnimationPlayer { Object.keys(this._startingStyles).forEach(prop => { const value = this._startingStyles ![prop]; if (value) { - this.element.style[prop] = value; + this.element.style.setProperty(prop, value); } else { this.element.style.removeProperty(prop); } diff --git a/packages/animations/browser/src/render/shared.ts b/packages/animations/browser/src/render/shared.ts index 916fce6746..7836992d17 100644 --- a/packages/animations/browser/src/render/shared.ts +++ b/packages/animations/browser/src/render/shared.ts @@ -203,3 +203,12 @@ export function getBodyNode(): any|null { export const matchesElement = _matches; export const containsElement = _contains; export const invokeQuery = _query; + +export function hypenatePropsObject(object: {[key: string]: any}): {[key: string]: any} { + const newObj: {[key: string]: any} = {}; + Object.keys(object).forEach(prop => { + const newProp = prop.replace(/([a-z])([A-Z])/g, '$1-$2'); + newObj[newProp] = object[prop]; + }); + return newObj; +} diff --git a/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts b/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts index 006ef7ae7f..4b9201d811 100644 --- a/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts +++ b/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts @@ -248,6 +248,61 @@ import {TestBed} from '../../testing'; assertStyle(element, 'width', '200px'); assertStyle(element, 'height', '50px'); }); + + it('should clean up 0 second animation styles (queried styles) that contain camel casing when complete', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
+
+
+ `, + animations: [ + trigger( + 'myAnimation', + [ + state('go', style({width: '200px'})), + transition( + '* => go', + [ + query('.foo', [style({maxHeight: '0px'})]), + query( + '.bar', + [ + style({width: '0px'}), + animate('1s', style({width: '100px'})), + ]), + ]), + ]), + ] + }) + 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; + + const elm = cmp.element.nativeElement; + const foo = elm.querySelector('.foo') as HTMLElement; + + cmp.myAnimationExp = 'go'; + fixture.detectChanges(); + + expect(foo.style.getPropertyValue('max-height')).toEqual('0px'); + + const player = engine.players.pop(); + player.finish(); + + expect(foo.style.getPropertyValue('max-height')).toBeFalsy(); + }); }); })();