Skip to content

Commit f983a6c

Browse files
matskotbosch
authored andcommitted
fix(animations): properly support boolean-based transitions and state changes (#19672)
Closes #9396 Closes #12337 PR Close #19672
1 parent 18f1b01 commit f983a6c

File tree

4 files changed

+145
-18
lines changed

4 files changed

+145
-18
lines changed

packages/animations/browser/src/dsl/animation_transition_expr.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,27 @@ function parseAnimationAlias(alias: string, errors: string[]): string {
5555
}
5656
}
5757

58+
const TRUE_BOOLEAN_VALUES = new Set<string>();
59+
TRUE_BOOLEAN_VALUES.add('true');
60+
TRUE_BOOLEAN_VALUES.add('1');
61+
62+
const FALSE_BOOLEAN_VALUES = new Set<string>();
63+
FALSE_BOOLEAN_VALUES.add('false');
64+
FALSE_BOOLEAN_VALUES.add('0');
65+
5866
function makeLambdaFromStates(lhs: string, rhs: string): TransitionMatcherFn {
67+
const LHS_MATCH_BOOLEAN = TRUE_BOOLEAN_VALUES.has(lhs) || FALSE_BOOLEAN_VALUES.has(lhs);
68+
const RHS_MATCH_BOOLEAN = TRUE_BOOLEAN_VALUES.has(rhs) || FALSE_BOOLEAN_VALUES.has(rhs);
69+
5970
return (fromState: any, toState: any): boolean => {
6071
let lhsMatch = lhs == ANY_STATE || lhs == fromState;
6172
let rhsMatch = rhs == ANY_STATE || rhs == toState;
6273

63-
if (!lhsMatch && typeof fromState === 'boolean') {
64-
lhsMatch = fromState ? lhs === 'true' : lhs === 'false';
74+
if (!lhsMatch && LHS_MATCH_BOOLEAN && typeof fromState === 'boolean') {
75+
lhsMatch = fromState ? TRUE_BOOLEAN_VALUES.has(lhs) : FALSE_BOOLEAN_VALUES.has(lhs);
6576
}
66-
if (!rhsMatch && typeof toState === 'boolean') {
67-
rhsMatch = toState ? rhs === 'true' : rhs === 'false';
77+
if (!rhsMatch && RHS_MATCH_BOOLEAN && typeof toState === 'boolean') {
78+
rhsMatch = toState ? TRUE_BOOLEAN_VALUES.has(rhs) : FALSE_BOOLEAN_VALUES.has(rhs);
6879
}
6980

7081
return lhsMatch && rhsMatch;

packages/animations/browser/src/render/transition_animation_engine.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,18 @@ export class AnimationTransitionNamespace {
119119

120120
listen(element: any, name: string, phase: string, callback: (event: any) => boolean): () => any {
121121
if (!this._triggers.hasOwnProperty(name)) {
122-
throw new Error(
123-
`Unable to listen on the animation trigger event "${phase}" because the animation trigger "${name}" doesn\'t exist!`);
122+
throw new Error(`Unable to listen on the animation trigger event "${
123+
phase}" because the animation trigger "${name}" doesn\'t exist!`);
124124
}
125125

126126
if (phase == null || phase.length == 0) {
127-
throw new Error(
128-
`Unable to listen on the animation trigger "${name}" because the provided event is undefined!`);
127+
throw new Error(`Unable to listen on the animation trigger "${
128+
name}" because the provided event is undefined!`);
129129
}
130130

131131
if (!isTriggerEventValid(phase)) {
132-
throw new Error(
133-
`The provided animation trigger event "${phase}" for the animation trigger "${name}" is not supported!`);
132+
throw new Error(`The provided animation trigger event "${phase}" for the animation trigger "${
133+
name}" is not supported!`);
134134
}
135135

136136
const listeners = getOrSetAsInMap(this._elementListeners, element, []);
@@ -802,7 +802,8 @@ export class TransitionAnimationEngine {
802802

803803
reportError(errors: string[]) {
804804
throw new Error(
805-
`Unable to process animations due to the following failed trigger transitions\n ${errors.join("\n")}`);
805+
`Unable to process animations due to the following failed trigger transitions\n ${
806+
errors.join('\n')}`);
806807
}
807808

808809
private _flushAnimations(cleanupFns: Function[], microtaskId: number):
@@ -1413,13 +1414,11 @@ function deleteOrUnsetInMap(map: Map<any, any[]>| {[key: string]: any}, key: any
14131414
return currentValues;
14141415
}
14151416

1416-
function normalizeTriggerValue(value: any): string {
1417-
switch (typeof value) {
1418-
case 'boolean':
1419-
return value ? '1' : '0';
1420-
default:
1421-
return value != null ? value.toString() : null;
1422-
}
1417+
function normalizeTriggerValue(value: any): any {
1418+
// we use `!= null` here because it's the most simple
1419+
// way to test against a "falsy" value without mixing
1420+
// in empty strings or a zero value. DO NOT OPTIMIZE.
1421+
return value != null ? value : null;
14231422
}
14241423

14251424
function isElementNode(node: any) {

packages/animations/src/animation_metadata.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ export interface AnimationStaggerMetadata extends AnimationMetadata {
257257
the
258258
* trigger is bound to (in the form of `[@triggerName]="expression"`.
259259
*
260+
* Animation trigger bindings strigify values and then match the previous and current values against
261+
* any linked transitions. If a boolean value is provided into the trigger binding then it will both
262+
* be represented as `1` or `true` and `0` or `false` for a true and false boolean values
263+
* respectively.
264+
*
260265
* ### Usage
261266
*
262267
* `trigger` will create an animation trigger reference based on the provided `name` value. The
@@ -734,6 +739,21 @@ export function keyframes(steps: AnimationStyleMetadata[]): AnimationKeyframesSe
734739
* ])
735740
* ```
736741
*
742+
* ### Boolean values
743+
* if a trigger binding value is a boolean value then it can be matched using a transition
744+
* expression that compares `true` and `false` or `1` and `0`.
745+
*
746+
* ```
747+
* // in the template
748+
* <div [@openClose]="open ? true : false">...</div>
749+
*
750+
* // in the component metadata
751+
* trigger('openClose', [
752+
* state('true', style({ height: '*' })),
753+
* state('false', style({ height: '0px' })),
754+
* transition('false <=> true', animate(500))
755+
* ])
756+
* ```
737757
* {@example core/animation/ts/dsl/animation_example.ts region='Component'}
738758
*
739759
* @experimental Animation support is experimental.

packages/core/test/animation/animation_integration_spec.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,103 @@ export function main() {
475475
]);
476476
});
477477

478+
it('should understand boolean values as `true` and `false` for transition animations', () => {
479+
@Component({
480+
selector: 'if-cmp',
481+
template: `
482+
<div [@myAnimation]="exp"></div>
483+
`,
484+
animations: [
485+
trigger(
486+
'myAnimation',
487+
[
488+
transition(
489+
'true => false',
490+
[
491+
style({opacity: 0}),
492+
animate(1234, style({opacity: 1})),
493+
]),
494+
transition(
495+
'false => true',
496+
[
497+
style({opacity: 1}),
498+
animate(4567, style({opacity: 0})),
499+
])
500+
]),
501+
]
502+
})
503+
class Cmp {
504+
exp: any = false;
505+
}
506+
507+
TestBed.configureTestingModule({declarations: [Cmp]});
508+
509+
const engine = TestBed.get(ɵAnimationEngine);
510+
const fixture = TestBed.createComponent(Cmp);
511+
const cmp = fixture.componentInstance;
512+
513+
cmp.exp = true;
514+
fixture.detectChanges();
515+
516+
cmp.exp = false;
517+
fixture.detectChanges();
518+
519+
let players = getLog();
520+
expect(players.length).toEqual(1);
521+
let [player] = players;
522+
523+
expect(player.duration).toEqual(1234);
524+
});
525+
526+
it('should understand boolean values as `true` and `false` for transition animations and apply the corresponding state() value',
527+
() => {
528+
@Component({
529+
selector: 'if-cmp',
530+
template: `
531+
<div [@myAnimation]="exp"></div>
532+
`,
533+
animations: [
534+
trigger(
535+
'myAnimation',
536+
[
537+
state('true', style({color: 'red'})),
538+
state('false', style({color: 'blue'})),
539+
transition(
540+
'true <=> false',
541+
[
542+
animate(1000, style({color: 'gold'})),
543+
animate(1000),
544+
]),
545+
]),
546+
]
547+
})
548+
class Cmp {
549+
exp: any = false;
550+
}
551+
552+
TestBed.configureTestingModule({declarations: [Cmp]});
553+
554+
const engine = TestBed.get(ɵAnimationEngine);
555+
const fixture = TestBed.createComponent(Cmp);
556+
const cmp = fixture.componentInstance;
557+
558+
cmp.exp = false;
559+
fixture.detectChanges();
560+
561+
cmp.exp = true;
562+
fixture.detectChanges();
563+
564+
let players = getLog();
565+
expect(players.length).toEqual(1);
566+
let [player] = players;
567+
568+
expect(player.keyframes).toEqual([
569+
{color: 'blue', offset: 0},
570+
{color: 'gold', offset: 0.5},
571+
{color: 'red', offset: 1},
572+
]);
573+
});
574+
478575
it('should not throw an error if a trigger with the same name exists in separate components',
479576
() => {
480577
@Component({selector: 'cmp1', template: '...', animations: [trigger('trig', [])]})

0 commit comments

Comments
 (0)