Skip to content

Commit 55860e1

Browse files
matskomatsko
committed
fix(animations): ensure AUTO styles are cleared at the end of the state-change animation
Closes angular#9014 Closes angular#9015
1 parent 4d51158 commit 55860e1

File tree

8 files changed

+86
-92
lines changed

8 files changed

+86
-92
lines changed

‎modules/@angular/compiler-cli/integrationtest/test/animate_spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ require('zone.js/dist/zone-node.js');
44
require('zone.js/dist/long-stack-trace-zone.js');
55

66
import {AnimateCmpNgFactory} from '../src/animate.ngfactory';
7-
import {AUTO_STYLE, ReflectiveInjector, DebugElement, getDebugNode} from '@angular/core';
7+
import {ReflectiveInjector, DebugElement, getDebugNode} from '@angular/core';
88
import {browserPlatform, BROWSER_APP_PROVIDERS} from '@angular/platform-browser';
99

1010
describe('template codegen output', () => {
@@ -26,7 +26,7 @@ describe('template codegen output', () => {
2626
comp.changeDetectorRef.detectChanges();
2727

2828
setTimeout(() => {
29-
expect(targetDebugElement.styles['height']).toEqual(AUTO_STYLE);
29+
expect(targetDebugElement.styles['height']).toEqual(null);
3030
expect(targetDebugElement.styles['borderColor']).toEqual('green');
3131
expect(targetDebugElement.styles['color']).toEqual('green');
3232

@@ -54,15 +54,15 @@ describe('template codegen output', () => {
5454
comp.changeDetectorRef.detectChanges();
5555

5656
setTimeout(() => {
57-
expect(targetDebugElement.styles['height']).toEqual(AUTO_STYLE);
57+
expect(targetDebugElement.styles['height']).toEqual(null);
5858
expect(targetDebugElement.styles['borderColor']).toEqual('black');
5959
expect(targetDebugElement.styles['color']).toEqual('black');
6060

6161
comp.instance.setAsClosed();
6262
comp.changeDetectorRef.detectChanges();
6363

6464
setTimeout(() => {
65-
expect(targetDebugElement.styles['height']).not.toEqual(AUTO_STYLE);
65+
expect(targetDebugElement.styles['height']).not.toEqual(null);
6666
expect(targetDebugElement.styles['borderColor']).not.toEqual('grey');
6767
expect(targetDebugElement.styles['color']).not.toEqual('grey');
6868
done();

‎modules/@angular/compiler/core_private.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ export var ANY_STATE = r.ANY_STATE;
8080
export var DEFAULT_STATE = r.DEFAULT_STATE;
8181
export var EMPTY_STATE = r.EMPTY_STATE;
8282
export var FILL_STYLE_FLAG = r.FILL_STYLE_FLAG;
83-
export var balanceAnimationStyles: typeof t.balanceAnimationStyles = r.balanceAnimationStyles;
83+
export var prepareFinalAnimationStyles: typeof t.prepareFinalAnimationStyles =
84+
r.prepareFinalAnimationStyles;
8485
export var balanceAnimationKeyframes: typeof t.balanceAnimationKeyframes =
8586
r.balanceAnimationKeyframes;
8687
export var flattenStyles: typeof t.flattenStyles = r.flattenStyles;

‎modules/@angular/compiler/src/animation/animation_compiler.ts‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
6868
}
6969

7070
visitAnimationStyles(ast: AnimationStylesAst, context: _AnimationBuilderContext): o.Expression {
71-
var stylesArr: any[] /** TODO #9100 */ = [];
71+
var stylesArr: any[] = [];
7272
if (context.isExpectingFirstStyleStep) {
7373
stylesArr.push(_ANIMATION_START_STATE_STYLES_VAR);
7474
context.isExpectingFirstStyleStep = false;
@@ -117,9 +117,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
117117
}
118118

119119
/** @internal */
120-
_callAnimateMethod(
121-
ast: AnimationStepAst, startingStylesExpr: any /** TODO #9100 */,
122-
keyframesExpr: any /** TODO #9100 */) {
120+
_callAnimateMethod(ast: AnimationStepAst, startingStylesExpr: any, keyframesExpr: any) {
123121
return _ANIMATION_FACTORY_RENDERER_VAR.callMethod('animate', [
124122
_ANIMATION_FACTORY_ELEMENT_VAR, startingStylesExpr, keyframesExpr, o.literal(ast.duration),
125123
o.literal(ast.delay), o.literal(ast.easing)
@@ -141,11 +139,8 @@ class _AnimationBuilder implements AnimationAstVisitor {
141139
visitAnimationStateDeclaration(
142140
ast: AnimationStateDeclarationAst, context: _AnimationBuilderContext): void {
143141
var flatStyles: {[key: string]: string | number} = {};
144-
_getStylesArray(ast).forEach((entry: any /** TODO #9100 */) => {
145-
StringMapWrapper.forEach(
146-
entry, (value: any /** TODO #9100 */, key: any /** TODO #9100 */) => {
147-
flatStyles[key] = value;
148-
});
142+
_getStylesArray(ast).forEach(entry => {
143+
StringMapWrapper.forEach(entry, (value: string, key: string) => { flatStyles[key] = value; });
149144
});
150145
context.stateMap.registerState(ast.stateName, flatStyles);
151146
}
@@ -160,7 +155,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
160155

161156
context.isExpectingFirstStyleStep = true;
162157

163-
var stateChangePreconditions: any[] /** TODO #9100 */ = [];
158+
var stateChangePreconditions: o.Expression[] = [];
164159

165160
ast.stateChanges.forEach(stateChange => {
166161
stateChangePreconditions.push(
@@ -192,7 +187,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
192187
// this should always be defined even if the user overrides it
193188
context.stateMap.registerState(DEFAULT_STATE, {});
194189

195-
var statements: any[] /** TODO #9100 */ = [];
190+
var statements: o.Statement[] = [];
196191
statements.push(_ANIMATION_FACTORY_VIEW_VAR
197192
.callMethod(
198193
'cancelActiveAnimation',
@@ -202,6 +197,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
202197
])
203198
.toStmt());
204199

200+
205201
statements.push(_ANIMATION_COLLECTED_STYLES.set(EMPTY_MAP).toDeclStmt());
206202
statements.push(_ANIMATION_PLAYER_VAR.set(o.NULL_EXPR).toDeclStmt());
207203

@@ -225,7 +221,6 @@ class _AnimationBuilder implements AnimationAstVisitor {
225221
_ANIMATION_END_STATE_STYLES_VAR.equals(o.NULL_EXPR),
226222
[_ANIMATION_END_STATE_STYLES_VAR.set(_ANIMATION_DEFAULT_STATE_VAR).toStmt()]));
227223

228-
229224
var RENDER_STYLES_FN = o.importExpr(Identifiers.renderStyles);
230225

231226
// before we start any animation we want to clear out the starting
@@ -259,7 +254,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
259254
[], [RENDER_STYLES_FN
260255
.callFn([
261256
_ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR,
262-
o.importExpr(Identifiers.balanceAnimationStyles).callFn([
257+
o.importExpr(Identifiers.prepareFinalAnimationStyles).callFn([
263258
_ANIMATION_START_STATE_STYLES_VAR, _ANIMATION_END_STATE_STYLES_VAR
264259
])
265260
])
@@ -292,17 +287,15 @@ class _AnimationBuilder implements AnimationAstVisitor {
292287
var fnStatement = ast.visit(this, context).toDeclStmt(this._fnVarName);
293288
var fnVariable = o.variable(this._fnVarName);
294289

295-
var lookupMap: any[] /** TODO #9100 */ = [];
290+
var lookupMap: any[] = [];
296291
StringMapWrapper.forEach(
297-
context.stateMap.states,
298-
(value: any /** TODO #9100 */, stateName: any /** TODO #9100 */) => {
292+
context.stateMap.states, (value: {[key: string]: string}, stateName: string) => {
299293
var variableValue = EMPTY_MAP;
300294
if (isPresent(value)) {
301-
let styleMap: any[] /** TODO #9100 */ = [];
302-
StringMapWrapper.forEach(
303-
value, (value: any /** TODO #9100 */, key: any /** TODO #9100 */) => {
304-
styleMap.push([key, o.literal(value)]);
305-
});
295+
let styleMap: any[] = [];
296+
StringMapWrapper.forEach(value, (value: string, key: string) => {
297+
styleMap.push([key, o.literal(value)]);
298+
});
306299
variableValue = o.literalMap(styleMap);
307300
}
308301
lookupMap.push([stateName, variableValue]);
@@ -356,6 +349,6 @@ function _isEndStateAnimateStep(step: AnimationAst): boolean {
356349
return false;
357350
}
358351

359-
function _getStylesArray(obj: any) {
352+
function _getStylesArray(obj: any): {[key: string]: any}[] {
360353
return obj.styles.styles;
361354
}

‎modules/@angular/compiler/src/identifiers.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {ChangeDetectionStrategy, ChangeDetectorRef, ElementRef, Injector, QueryList, RenderComponentType, Renderer, SimpleChange, TemplateRef, ViewContainerRef, ViewEncapsulation} from '@angular/core';
22

3-
import {AnimationGroupPlayer as AnimationGroupPlayer_, AnimationKeyframe as AnimationKeyframe_, AnimationSequencePlayer as AnimationSequencePlayer_, AnimationStyles as AnimationStyles_, AppElement, AppView, ChangeDetectorState, DebugAppView, DebugContext, EMPTY_ARRAY, EMPTY_MAP, NoOpAnimationPlayer as NoOpAnimationPlayer_, SecurityContext, StaticNodeDebugInfo, TemplateRef_, ValueUnwrapper, ViewType, ViewUtils, balanceAnimationKeyframes as impBalanceAnimationKeyframes, balanceAnimationStyles as impBalanceAnimationStyles, castByValue, checkBinding, clearStyles as impClearStyles, collectAndResolveStyles as impCollectAndResolveStyles, devModeEqual, flattenNestedViewRenderNodes, interpolate, pureProxy1, pureProxy10, pureProxy2, pureProxy3, pureProxy4, pureProxy5, pureProxy6, pureProxy7, pureProxy8, pureProxy9, renderStyles as impRenderStyles, uninitialized} from '../core_private';
3+
import {AnimationGroupPlayer as AnimationGroupPlayer_, AnimationKeyframe as AnimationKeyframe_, AnimationSequencePlayer as AnimationSequencePlayer_, AnimationStyles as AnimationStyles_, AppElement, AppView, ChangeDetectorState, DebugAppView, DebugContext, EMPTY_ARRAY, EMPTY_MAP, NoOpAnimationPlayer as NoOpAnimationPlayer_, SecurityContext, StaticNodeDebugInfo, TemplateRef_, ValueUnwrapper, ViewType, ViewUtils, balanceAnimationKeyframes as impBalanceAnimationKeyframes, castByValue, checkBinding, clearStyles as impClearStyles, collectAndResolveStyles as impCollectAndResolveStyles, devModeEqual, flattenNestedViewRenderNodes, interpolate, prepareFinalAnimationStyles as impBalanceAnimationStyles, pureProxy1, pureProxy10, pureProxy2, pureProxy3, pureProxy4, pureProxy5, pureProxy6, pureProxy7, pureProxy8, pureProxy9, renderStyles as impRenderStyles, uninitialized} from '../core_private';
44

55
import {CompileIdentifierMetadata, CompileTokenMetadata} from './compile_metadata';
66
import {assetUrl} from './util';
@@ -195,8 +195,8 @@ export class Identifiers {
195195
moduleUrl: assetUrl('core', 'animation/animation_sequence_player'),
196196
runtime: impAnimationSequencePlayer
197197
});
198-
static balanceAnimationStyles = new CompileIdentifierMetadata({
199-
name: 'balanceAnimationStyles',
198+
static prepareFinalAnimationStyles = new CompileIdentifierMetadata({
199+
name: 'prepareFinalAnimationStyles',
200200
moduleUrl: ANIMATION_STYLE_UTIL_ASSET_URL,
201201
runtime: impBalanceAnimationStyles
202202
});

‎modules/@angular/core/private_export.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export declare namespace __core_private_types__ {
110110
export var AnimationGroupPlayer: typeof AnimationGroupPlayer_;
111111
export type AnimationKeyframe = AnimationKeyframe_;
112112
export var AnimationKeyframe: typeof AnimationKeyframe_;
113-
export var balanceAnimationStyles: typeof animationUtils.balanceAnimationStyles;
113+
export var prepareFinalAnimationStyles: typeof animationUtils.prepareFinalAnimationStyles;
114114
export var balanceAnimationKeyframes: typeof animationUtils.balanceAnimationKeyframes;
115115
export var flattenStyles: typeof animationUtils.flattenStyles;
116116
export var clearStyles: typeof animationUtils.clearStyles;
@@ -181,7 +181,7 @@ export var __core_private__ = {
181181
AnimationSequencePlayer: AnimationSequencePlayer_,
182182
AnimationGroupPlayer: AnimationGroupPlayer_,
183183
AnimationKeyframe: AnimationKeyframe_,
184-
balanceAnimationStyles: animationUtils.balanceAnimationStyles,
184+
prepareFinalAnimationStyles: animationUtils.prepareFinalAnimationStyles,
185185
balanceAnimationKeyframes: animationUtils.balanceAnimationKeyframes,
186186
flattenStyles: animationUtils.flattenStyles,
187187
clearStyles: animationUtils.clearStyles,

‎modules/@angular/core/src/animation/animation_style_util.ts‎

Lines changed: 42 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,20 @@ import {isArray, isPresent} from '../facade/lang';
44
import {FILL_STYLE_FLAG} from './animation_constants';
55
import {AUTO_STYLE} from './metadata';
66

7-
export function balanceAnimationStyles(
7+
export function prepareFinalAnimationStyles(
88
previousStyles: {[key: string]: string | number}, newStyles: {[key: string]: string | number},
9-
nullValue: any /** TODO #9100 */ = null): {[key: string]: string} {
9+
nullValue: string = null): {[key: string]: string} {
1010
var finalStyles: {[key: string]: string} = {};
1111

12-
StringMapWrapper.forEach(
13-
newStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
14-
finalStyles[prop] = value.toString();
15-
});
12+
StringMapWrapper.forEach(newStyles, (value: string, prop: string) => {
13+
finalStyles[prop] = value == AUTO_STYLE ? nullValue : value.toString();
14+
});
1615

17-
StringMapWrapper.forEach(
18-
previousStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
19-
if (!isPresent(finalStyles[prop])) {
20-
finalStyles[prop] = nullValue;
21-
}
22-
});
16+
StringMapWrapper.forEach(previousStyles, (value: string, prop: string) => {
17+
if (!isPresent(finalStyles[prop])) {
18+
finalStyles[prop] = nullValue;
19+
}
20+
});
2321

2422
return finalStyles;
2523
}
@@ -33,18 +31,17 @@ export function balanceAnimationKeyframes(
3331
// phase 1: copy all the styles from the first keyframe into the lookup map
3432
var flatenedFirstKeyframeStyles = flattenStyles(firstKeyframe.styles.styles);
3533

36-
var extraFirstKeyframeStyles = {};
34+
var extraFirstKeyframeStyles: {[key: string]: string} = {};
3735
var hasExtraFirstStyles = false;
38-
StringMapWrapper.forEach(
39-
collectedStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
40-
// if the style is already defined in the first keyframe then
41-
// we do not replace it.
42-
if (!(flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop]) {
43-
(flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop] = value;
44-
(extraFirstKeyframeStyles as any /** TODO #9100 */)[prop] = value;
45-
hasExtraFirstStyles = true;
46-
}
47-
});
36+
StringMapWrapper.forEach(collectedStyles, (value: string, prop: string) => {
37+
// if the style is already defined in the first keyframe then
38+
// we do not replace it.
39+
if (!flatenedFirstKeyframeStyles[prop]) {
40+
flatenedFirstKeyframeStyles[prop] = value;
41+
extraFirstKeyframeStyles[prop] = value;
42+
hasExtraFirstStyles = true;
43+
}
44+
});
4845

4946
var keyframeCollectedStyles = StringMapWrapper.merge({}, flatenedFirstKeyframeStyles);
5047

@@ -53,27 +50,25 @@ export function balanceAnimationKeyframes(
5350
ListWrapper.insert(finalKeyframe.styles.styles, 0, finalStateStyles);
5451

5552
var flatenedFinalKeyframeStyles = flattenStyles(finalKeyframe.styles.styles);
56-
var extraFinalKeyframeStyles = {};
53+
var extraFinalKeyframeStyles: {[key: string]: string} = {};
5754
var hasExtraFinalStyles = false;
58-
StringMapWrapper.forEach(
59-
keyframeCollectedStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
60-
if (!isPresent((flatenedFinalKeyframeStyles as any /** TODO #9100 */)[prop])) {
61-
(extraFinalKeyframeStyles as any /** TODO #9100 */)[prop] = AUTO_STYLE;
62-
hasExtraFinalStyles = true;
63-
}
64-
});
55+
StringMapWrapper.forEach(keyframeCollectedStyles, (value: string, prop: string) => {
56+
if (!isPresent(flatenedFinalKeyframeStyles[prop])) {
57+
extraFinalKeyframeStyles[prop] = AUTO_STYLE;
58+
hasExtraFinalStyles = true;
59+
}
60+
});
6561

6662
if (hasExtraFinalStyles) {
6763
finalKeyframe.styles.styles.push(extraFinalKeyframeStyles);
6864
}
6965

70-
StringMapWrapper.forEach(
71-
flatenedFinalKeyframeStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
72-
if (!isPresent((flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop])) {
73-
(extraFirstKeyframeStyles as any /** TODO #9100 */)[prop] = AUTO_STYLE;
74-
hasExtraFirstStyles = true;
75-
}
76-
});
66+
StringMapWrapper.forEach(flatenedFinalKeyframeStyles, (value: string, prop: string) => {
67+
if (!isPresent(flatenedFirstKeyframeStyles[prop])) {
68+
extraFirstKeyframeStyles[prop] = AUTO_STYLE;
69+
hasExtraFirstStyles = true;
70+
}
71+
});
7772

7873
if (hasExtraFirstStyles) {
7974
firstKeyframe.styles.styles.push(extraFirstKeyframeStyles);
@@ -91,34 +86,32 @@ export function clearStyles(styles: {[key: string]: string | number}): {[key: st
9186
export function collectAndResolveStyles(
9287
collection: {[key: string]: string | number}, styles: {[key: string]: string | number}[]) {
9388
return styles.map(entry => {
94-
var stylesObj = {};
95-
StringMapWrapper.forEach(entry, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
89+
var stylesObj: {[key: string]: string | number} = {};
90+
StringMapWrapper.forEach(entry, (value: string | number, prop: string) => {
9691
if (value == FILL_STYLE_FLAG) {
9792
value = collection[prop];
9893
if (!isPresent(value)) {
9994
value = AUTO_STYLE;
10095
}
10196
}
10297
collection[prop] = value;
103-
(stylesObj as any /** TODO #9100 */)[prop] = value;
98+
stylesObj[prop] = value;
10499
});
105100
return stylesObj;
106101
});
107102
}
108103

109104
export function renderStyles(
110105
element: any, renderer: any, styles: {[key: string]: string | number}): void {
111-
StringMapWrapper.forEach(styles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
112-
renderer.setElementStyle(element, prop, value);
113-
});
106+
StringMapWrapper.forEach(
107+
styles, (value: string, prop: string) => { renderer.setElementStyle(element, prop, value); });
114108
}
115109

116-
export function flattenStyles(styles: {[key: string]: string | number}[]) {
117-
var finalStyles = {};
110+
export function flattenStyles(styles: {[key: string]: string | number}[]): {[key: string]: string} {
111+
var finalStyles: {[key: string]: string} = {};
118112
styles.forEach(entry => {
119-
StringMapWrapper.forEach(entry, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => {
120-
(finalStyles as any /** TODO #9100 */)[prop] = value;
121-
});
113+
StringMapWrapper.forEach(
114+
entry, (value: string, prop: string) => { finalStyles[prop] = value; });
122115
});
123116
return finalStyles;
124117
}

‎modules/@angular/core/test/animation/animation_style_util_spec.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,36 @@ import {AsyncTestCompleter, beforeEach, ddescribe, describe, expect, iit, inject
1414
export function main() {
1515
describe('Animation Style Utils', function() {
1616

17-
describe('balanceAnimationStyles', () => {
17+
describe('prepareFinalAnimationStyles', () => {
1818
it('should set all non-shared styles to the provided null value between the two sets of styles',
1919
() => {
2020
var styles = {opacity: 0, color: 'red'};
2121
var newStyles = {background: 'red'};
2222
var flag = '*';
23-
var result = animationUtils.balanceAnimationStyles(styles, newStyles, flag);
23+
var result = animationUtils.prepareFinalAnimationStyles(styles, newStyles, flag);
2424
expect(result).toEqual({opacity: flag, color: flag, background: 'red'})
2525
});
2626

2727
it('should handle an empty set of styles', () => {
2828
var value = '*';
2929

30-
expect(animationUtils.balanceAnimationStyles({}, {opacity: '0'}, value)).toEqual({
30+
expect(animationUtils.prepareFinalAnimationStyles({}, {opacity: '0'}, value)).toEqual({
3131
opacity: '0'
3232
});
3333

34-
expect(animationUtils.balanceAnimationStyles({opacity: '0'}, {}, value)).toEqual({
34+
expect(animationUtils.prepareFinalAnimationStyles({opacity: '0'}, {}, value)).toEqual({
3535
opacity: value
3636
});
3737
});
38+
39+
it('should set all AUTO styles to the null value', () => {
40+
var styles = {opacity: 0};
41+
var newStyles = {color: '*', border: '*'};
42+
var flag = '*';
43+
var result = animationUtils.prepareFinalAnimationStyles(styles, newStyles, null);
44+
expect(result).toEqual({opacity: null, color: null, border: null})
45+
});
46+
3847
});
3948

4049
describe('balanceAnimationKeyframes', () => {
@@ -91,12 +100,9 @@ export function main() {
91100

92101
describe('clearStyles', () => {
93102
it('should set all the style values to "null"', () => {
94-
var styles = {'opacity': 0, 'width': 100, 'color': 'red'};
95-
var expectedResult = {
96-
'opacity': null as number,
97-
'width': null as number,
98-
'color': null as string
99-
};
103+
var styles: {[key: string]: string | number} = {'opacity': 0, 'width': 100, 'color': 'red'};
104+
var expectedResult: {[key: string]:
105+
string | number} = {'opacity': null, 'width': null, 'color': null};
100106
expect(animationUtils.clearStyles(styles)).toEqual(expectedResult);
101107
});
102108

0 commit comments

Comments
 (0)