From f85b543cc1b82d3915f3cc1a7e2821ff8bc2469b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 6 Jul 2017 17:33:16 -0700 Subject: [PATCH] fix(animations): properly detect state transition changes for object literals --- .../src/render/transition_animation_engine.ts | 10 ++ .../transition_animation_engine_spec.ts | 2 +- .../animation/animation_integration_spec.ts | 97 +++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 9e654a94e8..21286b13f9 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -200,6 +200,16 @@ export class AnimationTransitionNamespace { return player; } + const isRemoval = toState.value === VOID_VALUE; + + // normally this isn't reached by here, however, if an object expression + // is passed in then it may be a new object each time. Comparing the value + // is important since that will stay the same despite there being a new object. + // The removal arc here is special cased because the same element is triggered + // twice in the event that it contains animations on the outer/inner portions + // of the host container + if (!isRemoval && fromState.value === toState.value) return; + const playersOnElement: TransitionAnimationPlayer[] = getOrSetAsInMap(this._engine.playersByElement, element, []); playersOnElement.forEach(player => { diff --git a/packages/animations/browser/test/render/transition_animation_engine_spec.ts b/packages/animations/browser/test/render/transition_animation_engine_spec.ts index 2ed18f0047..df24cb2cdc 100644 --- a/packages/animations/browser/test/render/transition_animation_engine_spec.ts +++ b/packages/animations/browser/test/render/transition_animation_engine_spec.ts @@ -132,7 +132,7 @@ export function main() { engine.destroy(DEFAULT_NAMESPACE_ID, null); registerTrigger(element, engine, trig); - setProperty(element, engine, 'myTrigger', 'value'); + setProperty(element, engine, 'myTrigger', 'value2'); engine.flush(); expect((engine.players[0].getRealPlayer() as MockAnimationPlayer).duration) .toEqual(1234); diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 34284a940a..30470b3fb7 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -365,6 +365,14 @@ export function main() { const engine = TestBed.get(ɵAnimationEngine); const fixture = TestBed.createComponent(Cmp); const cmp = fixture.componentInstance; + + function resetState() { + cmp.exp2 = 'something'; + fixture.detectChanges(); + engine.flush(); + resetLog(); + } + cmp.exp1 = true; cmp.exp2 = null; @@ -375,6 +383,7 @@ export function main() { {offset: 0, width: '0px'}, {offset: 1, width: '100px'} ]); + resetState(); cmp.exp2 = false; fixture.detectChanges(); @@ -384,6 +393,7 @@ export function main() { {offset: 0, height: '0px'}, {offset: 1, height: '100px'} ]); + resetState(); cmp.exp2 = 0; fixture.detectChanges(); @@ -393,6 +403,7 @@ export function main() { {offset: 0, height: '0px'}, {offset: 1, height: '100px'} ]); + resetState(); cmp.exp2 = ''; fixture.detectChanges(); @@ -402,6 +413,7 @@ export function main() { {offset: 0, height: '0px'}, {offset: 1, height: '100px'} ]); + resetState(); cmp.exp2 = undefined; fixture.detectChanges(); @@ -411,6 +423,7 @@ export function main() { {offset: 0, height: '0px'}, {offset: 1, height: '100px'} ]); + resetState(); cmp.exp1 = false; cmp.exp2 = 'abc'; @@ -1279,6 +1292,90 @@ export function main() { expect(p.contains(c2)).toBeTruthy(); }); + it('should detect trigger changes based on object.value properties', () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition('* => 1', [animate(1234, style({opacity: 0}))]), + transition('* => 2', [animate(5678, style({opacity: 0}))]), + ]), + ] + }) + class Cmp { + public exp: any; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = '1'; + fixture.detectChanges(); + engine.flush(); + let players = getLog(); + expect(players.length).toEqual(1); + expect(players[0].duration).toEqual(1234); + resetLog(); + + cmp.exp = '2'; + fixture.detectChanges(); + engine.flush(); + players = getLog(); + expect(players.length).toEqual(1); + expect(players[0].duration).toEqual(5678); + }); + + it('should not render animations when the object expression value is the same as it was previously', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition('* => *', [animate(1234, style({opacity: 0}))]), + ]), + ] + }) + class Cmp { + public exp: any; + public params: any; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = '1'; + cmp.params = {}; + fixture.detectChanges(); + engine.flush(); + let players = getLog(); + expect(players.length).toEqual(1); + expect(players[0].duration).toEqual(1234); + resetLog(); + + cmp.exp = '1'; + cmp.params = {}; + fixture.detectChanges(); + engine.flush(); + players = getLog(); + expect(players.length).toEqual(0); + }); + it('should substitute in values if the provided state match is an object with values', () => { @Component({ selector: 'ani-cmp',