From a8920eb774346d809e03d992c454de18664b1be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 19 Sep 2017 14:22:22 -0700 Subject: [PATCH] fix(animations): properly support boolean-based transitions and state changes (#19279) Closes #9396 Closes #12337 PR Close #19279 --- .../src/dsl/animation_transition_expr.ts | 19 +++- .../src/render/transition_animation_engine.ts | 27 +++--- packages/animations/src/animation_metadata.ts | 21 ++++ .../animation/animation_integration_spec.ts | 97 +++++++++++++++++++ 4 files changed, 146 insertions(+), 18 deletions(-) diff --git a/packages/animations/browser/src/dsl/animation_transition_expr.ts b/packages/animations/browser/src/dsl/animation_transition_expr.ts index 23e3cdfc52..afad801158 100644 --- a/packages/animations/browser/src/dsl/animation_transition_expr.ts +++ b/packages/animations/browser/src/dsl/animation_transition_expr.ts @@ -65,16 +65,27 @@ function parseAnimationAlias(alias: string, errors: string[]): string|Transition } } +const TRUE_BOOLEAN_VALUES = new Set(); +TRUE_BOOLEAN_VALUES.add('true'); +TRUE_BOOLEAN_VALUES.add('1'); + +const FALSE_BOOLEAN_VALUES = new Set(); +FALSE_BOOLEAN_VALUES.add('false'); +FALSE_BOOLEAN_VALUES.add('0'); + function makeLambdaFromStates(lhs: string, rhs: string): TransitionMatcherFn { + const LHS_MATCH_BOOLEAN = TRUE_BOOLEAN_VALUES.has(lhs) || FALSE_BOOLEAN_VALUES.has(lhs); + const RHS_MATCH_BOOLEAN = TRUE_BOOLEAN_VALUES.has(rhs) || FALSE_BOOLEAN_VALUES.has(rhs); + return (fromState: any, toState: any): boolean => { let lhsMatch = lhs == ANY_STATE || lhs == fromState; let rhsMatch = rhs == ANY_STATE || rhs == toState; - if (!lhsMatch && typeof fromState === 'boolean') { - lhsMatch = fromState ? lhs === 'true' : lhs === 'false'; + if (!lhsMatch && LHS_MATCH_BOOLEAN && typeof fromState === 'boolean') { + lhsMatch = fromState ? TRUE_BOOLEAN_VALUES.has(lhs) : FALSE_BOOLEAN_VALUES.has(lhs); } - if (!rhsMatch && typeof toState === 'boolean') { - rhsMatch = toState ? rhs === 'true' : rhs === 'false'; + if (!rhsMatch && RHS_MATCH_BOOLEAN && typeof toState === 'boolean') { + rhsMatch = toState ? TRUE_BOOLEAN_VALUES.has(rhs) : FALSE_BOOLEAN_VALUES.has(rhs); } return lhsMatch && rhsMatch; diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 605093be17..23c35ac23a 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -119,18 +119,18 @@ export class AnimationTransitionNamespace { listen(element: any, name: string, phase: string, callback: (event: any) => boolean): () => any { if (!this._triggers.hasOwnProperty(name)) { - throw new Error( - `Unable to listen on the animation trigger event "${phase}" because the animation trigger "${name}" doesn\'t exist!`); + throw new Error(`Unable to listen on the animation trigger event "${ + phase}" because the animation trigger "${name}" doesn\'t exist!`); } if (phase == null || phase.length == 0) { - throw new Error( - `Unable to listen on the animation trigger "${name}" because the provided event is undefined!`); + throw new Error(`Unable to listen on the animation trigger "${ + name}" because the provided event is undefined!`); } if (!isTriggerEventValid(phase)) { - throw new Error( - `The provided animation trigger event "${phase}" for the animation trigger "${name}" is not supported!`); + throw new Error(`The provided animation trigger event "${phase}" for the animation trigger "${ + name}" is not supported!`); } const listeners = getOrSetAsInMap(this._elementListeners, element, []); @@ -838,7 +838,8 @@ export class TransitionAnimationEngine { reportError(errors: string[]) { throw new Error( - `Unable to process animations due to the following failed trigger transitions\n ${errors.join("\n")}`); + `Unable to process animations due to the following failed trigger transitions\n ${ + errors.join('\n')}`); } private _flushAnimations(cleanupFns: Function[], microtaskId: number): @@ -1447,13 +1448,11 @@ function deleteOrUnsetInMap(map: Map| {[key: string]: any}, key: any return currentValues; } -function normalizeTriggerValue(value: any): string { - switch (typeof value) { - case 'boolean': - return value ? '1' : '0'; - default: - return value != null ? value.toString() : null; - } +function normalizeTriggerValue(value: any): any { + // we use `!= null` here because it's the most simple + // way to test against a "falsy" value without mixing + // in empty strings or a zero value. DO NOT OPTIMIZE. + return value != null ? value : null; } function isElementNode(node: any) { diff --git a/packages/animations/src/animation_metadata.ts b/packages/animations/src/animation_metadata.ts index aaac524a82..d506750c65 100755 --- a/packages/animations/src/animation_metadata.ts +++ b/packages/animations/src/animation_metadata.ts @@ -257,6 +257,11 @@ export interface AnimationStaggerMetadata extends AnimationMetadata { the * trigger is bound to (in the form of `[@triggerName]="expression"`. * + * Animation trigger bindings strigify values and then match the previous and current values against + * any linked transitions. If a boolean value is provided into the trigger binding then it will both + * be represented as `1` or `true` and `0` or `false` for a true and false boolean values + * respectively. + * * ### Usage * * `trigger` will create an animation trigger reference based on the provided `name` value. The @@ -734,6 +739,22 @@ export function keyframes(steps: AnimationStyleMetadata[]): AnimationKeyframesSe * ]) * ``` * + * ### Boolean values + * if a trigger binding value is a boolean value then it can be matched using a transition + * expression that compares `true` and `false` or `1` and `0`. + * + * ``` + * // in the template + *
...
+ * + * // in the component metadata + * trigger('openClose', [ + * state('true', style({ height: '*' })), + * state('false', style({ height: '0px' })), + * transition('false <=> true', animate(500)) + * ]) + * ``` + * * ### Using :increment and :decrement * In addition to the :enter and :leave transition aliases, the :increment and :decrement aliases * can be used to kick off a transition when a numeric value has increased or decreased in value. diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 51932332f9..0f6b96059b 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -475,6 +475,103 @@ export function main() { ]); }); + it('should understand boolean values as `true` and `false` for transition animations', () => { + @Component({ + selector: 'if-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition( + 'true => false', + [ + style({opacity: 0}), + animate(1234, style({opacity: 1})), + ]), + transition( + 'false => true', + [ + style({opacity: 1}), + animate(4567, style({opacity: 0})), + ]) + ]), + ] + }) + class Cmp { + exp: any = false; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = true; + fixture.detectChanges(); + + cmp.exp = false; + fixture.detectChanges(); + + let players = getLog(); + expect(players.length).toEqual(1); + let [player] = players; + + expect(player.duration).toEqual(1234); + }); + + it('should understand boolean values as `true` and `false` for transition animations and apply the corresponding state() value', + () => { + @Component({ + selector: 'if-cmp', + template: ` +
+ `, + animations: [ + trigger( + 'myAnimation', + [ + state('true', style({color: 'red'})), + state('false', style({color: 'blue'})), + transition( + 'true <=> false', + [ + animate(1000, style({color: 'gold'})), + animate(1000), + ]), + ]), + ] + }) + class Cmp { + exp: any = false; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = false; + fixture.detectChanges(); + + cmp.exp = true; + fixture.detectChanges(); + + let players = getLog(); + expect(players.length).toEqual(1); + let [player] = players; + + expect(player.keyframes).toEqual([ + {color: 'blue', offset: 0}, + {color: 'gold', offset: 0.5}, + {color: 'red', offset: 1}, + ]); + }); + it('should not throw an error if a trigger with the same name exists in separate components', () => { @Component({selector: 'cmp1', template: '...', animations: [trigger('trig', [])]})