fix(animations): properly support boolean-based transitions and state…

… changes Closes angular#9396 Closes angular#12337
matsko · Sep 19, 2017 · c809202 · c809202
1 parent b6b18c1
commit c809202
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 11 deletions.
diff --git 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<string>();
+TRUE_BOOLEAN_VALUES.add('true');
+TRUE_BOOLEAN_VALUES.add('1');
+
+const FALSE_BOOLEAN_VALUES = new Set<string>();
+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
@@ -1413,13 +1413,8 @@ function deleteOrUnsetInMap(map: Map<any, any[]>| {[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 {
+  return value != null ? value : null;
 }
 
 function isElementNode(node: any) {

diff --git 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: `
+          <div [@myAnimation]="exp"></div>
+        `,
+          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: `
+          <div [@myAnimation]="exp"></div>
+        `,
+             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', [])]})