From 55860e1621635d916200ebfed1c89dfeb8754f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 3 Jun 2016 18:27:34 -0700 Subject: [PATCH] fix(animations): ensure AUTO styles are cleared at the end of the state-change animation Closes #9014 Closes #9015 --- .../integrationtest/test/animate_spec.ts | 8 +- modules/@angular/compiler/core_private.ts | 3 +- .../src/animation/animation_compiler.ts | 37 +++----- modules/@angular/compiler/src/identifiers.ts | 6 +- modules/@angular/core/private_export.ts | 4 +- .../src/animation/animation_style_util.ts | 91 +++++++++---------- .../animation/animation_style_util_spec.ts | 26 ++++-- .../@angular/platform-browser/core_private.ts | 3 +- 8 files changed, 86 insertions(+), 92 deletions(-) diff --git a/modules/@angular/compiler-cli/integrationtest/test/animate_spec.ts b/modules/@angular/compiler-cli/integrationtest/test/animate_spec.ts index 4b8aa83c0f..bb0da3636f 100644 --- a/modules/@angular/compiler-cli/integrationtest/test/animate_spec.ts +++ b/modules/@angular/compiler-cli/integrationtest/test/animate_spec.ts @@ -4,7 +4,7 @@ require('zone.js/dist/zone-node.js'); require('zone.js/dist/long-stack-trace-zone.js'); import {AnimateCmpNgFactory} from '../src/animate.ngfactory'; -import {AUTO_STYLE, ReflectiveInjector, DebugElement, getDebugNode} from '@angular/core'; +import {ReflectiveInjector, DebugElement, getDebugNode} from '@angular/core'; import {browserPlatform, BROWSER_APP_PROVIDERS} from '@angular/platform-browser'; describe('template codegen output', () => { @@ -26,7 +26,7 @@ describe('template codegen output', () => { comp.changeDetectorRef.detectChanges(); setTimeout(() => { - expect(targetDebugElement.styles['height']).toEqual(AUTO_STYLE); + expect(targetDebugElement.styles['height']).toEqual(null); expect(targetDebugElement.styles['borderColor']).toEqual('green'); expect(targetDebugElement.styles['color']).toEqual('green'); @@ -54,7 +54,7 @@ describe('template codegen output', () => { comp.changeDetectorRef.detectChanges(); setTimeout(() => { - expect(targetDebugElement.styles['height']).toEqual(AUTO_STYLE); + expect(targetDebugElement.styles['height']).toEqual(null); expect(targetDebugElement.styles['borderColor']).toEqual('black'); expect(targetDebugElement.styles['color']).toEqual('black'); @@ -62,7 +62,7 @@ describe('template codegen output', () => { comp.changeDetectorRef.detectChanges(); setTimeout(() => { - expect(targetDebugElement.styles['height']).not.toEqual(AUTO_STYLE); + expect(targetDebugElement.styles['height']).not.toEqual(null); expect(targetDebugElement.styles['borderColor']).not.toEqual('grey'); expect(targetDebugElement.styles['color']).not.toEqual('grey'); done(); diff --git a/modules/@angular/compiler/core_private.ts b/modules/@angular/compiler/core_private.ts index 359a54dcd0..c5255b4128 100644 --- a/modules/@angular/compiler/core_private.ts +++ b/modules/@angular/compiler/core_private.ts @@ -80,7 +80,8 @@ export var ANY_STATE = r.ANY_STATE; export var DEFAULT_STATE = r.DEFAULT_STATE; export var EMPTY_STATE = r.EMPTY_STATE; export var FILL_STYLE_FLAG = r.FILL_STYLE_FLAG; -export var balanceAnimationStyles: typeof t.balanceAnimationStyles = r.balanceAnimationStyles; +export var prepareFinalAnimationStyles: typeof t.prepareFinalAnimationStyles = + r.prepareFinalAnimationStyles; export var balanceAnimationKeyframes: typeof t.balanceAnimationKeyframes = r.balanceAnimationKeyframes; export var flattenStyles: typeof t.flattenStyles = r.flattenStyles; diff --git a/modules/@angular/compiler/src/animation/animation_compiler.ts b/modules/@angular/compiler/src/animation/animation_compiler.ts index bec0cabf43..7c367342eb 100644 --- a/modules/@angular/compiler/src/animation/animation_compiler.ts +++ b/modules/@angular/compiler/src/animation/animation_compiler.ts @@ -68,7 +68,7 @@ class _AnimationBuilder implements AnimationAstVisitor { } visitAnimationStyles(ast: AnimationStylesAst, context: _AnimationBuilderContext): o.Expression { - var stylesArr: any[] /** TODO #9100 */ = []; + var stylesArr: any[] = []; if (context.isExpectingFirstStyleStep) { stylesArr.push(_ANIMATION_START_STATE_STYLES_VAR); context.isExpectingFirstStyleStep = false; @@ -117,9 +117,7 @@ class _AnimationBuilder implements AnimationAstVisitor { } /** @internal */ - _callAnimateMethod( - ast: AnimationStepAst, startingStylesExpr: any /** TODO #9100 */, - keyframesExpr: any /** TODO #9100 */) { + _callAnimateMethod(ast: AnimationStepAst, startingStylesExpr: any, keyframesExpr: any) { return _ANIMATION_FACTORY_RENDERER_VAR.callMethod('animate', [ _ANIMATION_FACTORY_ELEMENT_VAR, startingStylesExpr, keyframesExpr, o.literal(ast.duration), o.literal(ast.delay), o.literal(ast.easing) @@ -141,11 +139,8 @@ class _AnimationBuilder implements AnimationAstVisitor { visitAnimationStateDeclaration( ast: AnimationStateDeclarationAst, context: _AnimationBuilderContext): void { var flatStyles: {[key: string]: string | number} = {}; - _getStylesArray(ast).forEach((entry: any /** TODO #9100 */) => { - StringMapWrapper.forEach( - entry, (value: any /** TODO #9100 */, key: any /** TODO #9100 */) => { - flatStyles[key] = value; - }); + _getStylesArray(ast).forEach(entry => { + StringMapWrapper.forEach(entry, (value: string, key: string) => { flatStyles[key] = value; }); }); context.stateMap.registerState(ast.stateName, flatStyles); } @@ -160,7 +155,7 @@ class _AnimationBuilder implements AnimationAstVisitor { context.isExpectingFirstStyleStep = true; - var stateChangePreconditions: any[] /** TODO #9100 */ = []; + var stateChangePreconditions: o.Expression[] = []; ast.stateChanges.forEach(stateChange => { stateChangePreconditions.push( @@ -192,7 +187,7 @@ class _AnimationBuilder implements AnimationAstVisitor { // this should always be defined even if the user overrides it context.stateMap.registerState(DEFAULT_STATE, {}); - var statements: any[] /** TODO #9100 */ = []; + var statements: o.Statement[] = []; statements.push(_ANIMATION_FACTORY_VIEW_VAR .callMethod( 'cancelActiveAnimation', @@ -202,6 +197,7 @@ class _AnimationBuilder implements AnimationAstVisitor { ]) .toStmt()); + statements.push(_ANIMATION_COLLECTED_STYLES.set(EMPTY_MAP).toDeclStmt()); statements.push(_ANIMATION_PLAYER_VAR.set(o.NULL_EXPR).toDeclStmt()); @@ -225,7 +221,6 @@ class _AnimationBuilder implements AnimationAstVisitor { _ANIMATION_END_STATE_STYLES_VAR.equals(o.NULL_EXPR), [_ANIMATION_END_STATE_STYLES_VAR.set(_ANIMATION_DEFAULT_STATE_VAR).toStmt()])); - var RENDER_STYLES_FN = o.importExpr(Identifiers.renderStyles); // before we start any animation we want to clear out the starting @@ -259,7 +254,7 @@ class _AnimationBuilder implements AnimationAstVisitor { [], [RENDER_STYLES_FN .callFn([ _ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR, - o.importExpr(Identifiers.balanceAnimationStyles).callFn([ + o.importExpr(Identifiers.prepareFinalAnimationStyles).callFn([ _ANIMATION_START_STATE_STYLES_VAR, _ANIMATION_END_STATE_STYLES_VAR ]) ]) @@ -292,17 +287,15 @@ class _AnimationBuilder implements AnimationAstVisitor { var fnStatement = ast.visit(this, context).toDeclStmt(this._fnVarName); var fnVariable = o.variable(this._fnVarName); - var lookupMap: any[] /** TODO #9100 */ = []; + var lookupMap: any[] = []; StringMapWrapper.forEach( - context.stateMap.states, - (value: any /** TODO #9100 */, stateName: any /** TODO #9100 */) => { + context.stateMap.states, (value: {[key: string]: string}, stateName: string) => { var variableValue = EMPTY_MAP; if (isPresent(value)) { - let styleMap: any[] /** TODO #9100 */ = []; - StringMapWrapper.forEach( - value, (value: any /** TODO #9100 */, key: any /** TODO #9100 */) => { - styleMap.push([key, o.literal(value)]); - }); + let styleMap: any[] = []; + StringMapWrapper.forEach(value, (value: string, key: string) => { + styleMap.push([key, o.literal(value)]); + }); variableValue = o.literalMap(styleMap); } lookupMap.push([stateName, variableValue]); @@ -356,6 +349,6 @@ function _isEndStateAnimateStep(step: AnimationAst): boolean { return false; } -function _getStylesArray(obj: any) { +function _getStylesArray(obj: any): {[key: string]: any}[] { return obj.styles.styles; } diff --git a/modules/@angular/compiler/src/identifiers.ts b/modules/@angular/compiler/src/identifiers.ts index 47561a42c1..c000fcfd54 100644 --- a/modules/@angular/compiler/src/identifiers.ts +++ b/modules/@angular/compiler/src/identifiers.ts @@ -1,6 +1,6 @@ import {ChangeDetectionStrategy, ChangeDetectorRef, ElementRef, Injector, QueryList, RenderComponentType, Renderer, SimpleChange, TemplateRef, ViewContainerRef, ViewEncapsulation} from '@angular/core'; -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'; +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'; import {CompileIdentifierMetadata, CompileTokenMetadata} from './compile_metadata'; import {assetUrl} from './util'; @@ -195,8 +195,8 @@ export class Identifiers { moduleUrl: assetUrl('core', 'animation/animation_sequence_player'), runtime: impAnimationSequencePlayer }); - static balanceAnimationStyles = new CompileIdentifierMetadata({ - name: 'balanceAnimationStyles', + static prepareFinalAnimationStyles = new CompileIdentifierMetadata({ + name: 'prepareFinalAnimationStyles', moduleUrl: ANIMATION_STYLE_UTIL_ASSET_URL, runtime: impBalanceAnimationStyles }); diff --git a/modules/@angular/core/private_export.ts b/modules/@angular/core/private_export.ts index b3cdf516bd..c61a90961b 100644 --- a/modules/@angular/core/private_export.ts +++ b/modules/@angular/core/private_export.ts @@ -110,7 +110,7 @@ export declare namespace __core_private_types__ { export var AnimationGroupPlayer: typeof AnimationGroupPlayer_; export type AnimationKeyframe = AnimationKeyframe_; export var AnimationKeyframe: typeof AnimationKeyframe_; - export var balanceAnimationStyles: typeof animationUtils.balanceAnimationStyles; + export var prepareFinalAnimationStyles: typeof animationUtils.prepareFinalAnimationStyles; export var balanceAnimationKeyframes: typeof animationUtils.balanceAnimationKeyframes; export var flattenStyles: typeof animationUtils.flattenStyles; export var clearStyles: typeof animationUtils.clearStyles; @@ -181,7 +181,7 @@ export var __core_private__ = { AnimationSequencePlayer: AnimationSequencePlayer_, AnimationGroupPlayer: AnimationGroupPlayer_, AnimationKeyframe: AnimationKeyframe_, - balanceAnimationStyles: animationUtils.balanceAnimationStyles, + prepareFinalAnimationStyles: animationUtils.prepareFinalAnimationStyles, balanceAnimationKeyframes: animationUtils.balanceAnimationKeyframes, flattenStyles: animationUtils.flattenStyles, clearStyles: animationUtils.clearStyles, diff --git a/modules/@angular/core/src/animation/animation_style_util.ts b/modules/@angular/core/src/animation/animation_style_util.ts index fdbb70fc88..d9ba2e461a 100644 --- a/modules/@angular/core/src/animation/animation_style_util.ts +++ b/modules/@angular/core/src/animation/animation_style_util.ts @@ -4,22 +4,20 @@ import {isArray, isPresent} from '../facade/lang'; import {FILL_STYLE_FLAG} from './animation_constants'; import {AUTO_STYLE} from './metadata'; -export function balanceAnimationStyles( +export function prepareFinalAnimationStyles( previousStyles: {[key: string]: string | number}, newStyles: {[key: string]: string | number}, - nullValue: any /** TODO #9100 */ = null): {[key: string]: string} { + nullValue: string = null): {[key: string]: string} { var finalStyles: {[key: string]: string} = {}; - StringMapWrapper.forEach( - newStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - finalStyles[prop] = value.toString(); - }); + StringMapWrapper.forEach(newStyles, (value: string, prop: string) => { + finalStyles[prop] = value == AUTO_STYLE ? nullValue : value.toString(); + }); - StringMapWrapper.forEach( - previousStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - if (!isPresent(finalStyles[prop])) { - finalStyles[prop] = nullValue; - } - }); + StringMapWrapper.forEach(previousStyles, (value: string, prop: string) => { + if (!isPresent(finalStyles[prop])) { + finalStyles[prop] = nullValue; + } + }); return finalStyles; } @@ -33,18 +31,17 @@ export function balanceAnimationKeyframes( // phase 1: copy all the styles from the first keyframe into the lookup map var flatenedFirstKeyframeStyles = flattenStyles(firstKeyframe.styles.styles); - var extraFirstKeyframeStyles = {}; + var extraFirstKeyframeStyles: {[key: string]: string} = {}; var hasExtraFirstStyles = false; - StringMapWrapper.forEach( - collectedStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - // if the style is already defined in the first keyframe then - // we do not replace it. - if (!(flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop]) { - (flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop] = value; - (extraFirstKeyframeStyles as any /** TODO #9100 */)[prop] = value; - hasExtraFirstStyles = true; - } - }); + StringMapWrapper.forEach(collectedStyles, (value: string, prop: string) => { + // if the style is already defined in the first keyframe then + // we do not replace it. + if (!flatenedFirstKeyframeStyles[prop]) { + flatenedFirstKeyframeStyles[prop] = value; + extraFirstKeyframeStyles[prop] = value; + hasExtraFirstStyles = true; + } + }); var keyframeCollectedStyles = StringMapWrapper.merge({}, flatenedFirstKeyframeStyles); @@ -53,27 +50,25 @@ export function balanceAnimationKeyframes( ListWrapper.insert(finalKeyframe.styles.styles, 0, finalStateStyles); var flatenedFinalKeyframeStyles = flattenStyles(finalKeyframe.styles.styles); - var extraFinalKeyframeStyles = {}; + var extraFinalKeyframeStyles: {[key: string]: string} = {}; var hasExtraFinalStyles = false; - StringMapWrapper.forEach( - keyframeCollectedStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - if (!isPresent((flatenedFinalKeyframeStyles as any /** TODO #9100 */)[prop])) { - (extraFinalKeyframeStyles as any /** TODO #9100 */)[prop] = AUTO_STYLE; - hasExtraFinalStyles = true; - } - }); + StringMapWrapper.forEach(keyframeCollectedStyles, (value: string, prop: string) => { + if (!isPresent(flatenedFinalKeyframeStyles[prop])) { + extraFinalKeyframeStyles[prop] = AUTO_STYLE; + hasExtraFinalStyles = true; + } + }); if (hasExtraFinalStyles) { finalKeyframe.styles.styles.push(extraFinalKeyframeStyles); } - StringMapWrapper.forEach( - flatenedFinalKeyframeStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - if (!isPresent((flatenedFirstKeyframeStyles as any /** TODO #9100 */)[prop])) { - (extraFirstKeyframeStyles as any /** TODO #9100 */)[prop] = AUTO_STYLE; - hasExtraFirstStyles = true; - } - }); + StringMapWrapper.forEach(flatenedFinalKeyframeStyles, (value: string, prop: string) => { + if (!isPresent(flatenedFirstKeyframeStyles[prop])) { + extraFirstKeyframeStyles[prop] = AUTO_STYLE; + hasExtraFirstStyles = true; + } + }); if (hasExtraFirstStyles) { firstKeyframe.styles.styles.push(extraFirstKeyframeStyles); @@ -91,8 +86,8 @@ export function clearStyles(styles: {[key: string]: string | number}): {[key: st export function collectAndResolveStyles( collection: {[key: string]: string | number}, styles: {[key: string]: string | number}[]) { return styles.map(entry => { - var stylesObj = {}; - StringMapWrapper.forEach(entry, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { + var stylesObj: {[key: string]: string | number} = {}; + StringMapWrapper.forEach(entry, (value: string | number, prop: string) => { if (value == FILL_STYLE_FLAG) { value = collection[prop]; if (!isPresent(value)) { @@ -100,7 +95,7 @@ export function collectAndResolveStyles( } } collection[prop] = value; - (stylesObj as any /** TODO #9100 */)[prop] = value; + stylesObj[prop] = value; }); return stylesObj; }); @@ -108,17 +103,15 @@ export function collectAndResolveStyles( export function renderStyles( element: any, renderer: any, styles: {[key: string]: string | number}): void { - StringMapWrapper.forEach(styles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - renderer.setElementStyle(element, prop, value); - }); + StringMapWrapper.forEach( + styles, (value: string, prop: string) => { renderer.setElementStyle(element, prop, value); }); } -export function flattenStyles(styles: {[key: string]: string | number}[]) { - var finalStyles = {}; +export function flattenStyles(styles: {[key: string]: string | number}[]): {[key: string]: string} { + var finalStyles: {[key: string]: string} = {}; styles.forEach(entry => { - StringMapWrapper.forEach(entry, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - (finalStyles as any /** TODO #9100 */)[prop] = value; - }); + StringMapWrapper.forEach( + entry, (value: string, prop: string) => { finalStyles[prop] = value; }); }); return finalStyles; } diff --git a/modules/@angular/core/test/animation/animation_style_util_spec.ts b/modules/@angular/core/test/animation/animation_style_util_spec.ts index 7d359dd3f7..e78a556d5e 100644 --- a/modules/@angular/core/test/animation/animation_style_util_spec.ts +++ b/modules/@angular/core/test/animation/animation_style_util_spec.ts @@ -14,27 +14,36 @@ import {AsyncTestCompleter, beforeEach, ddescribe, describe, expect, iit, inject export function main() { describe('Animation Style Utils', function() { - describe('balanceAnimationStyles', () => { + describe('prepareFinalAnimationStyles', () => { it('should set all non-shared styles to the provided null value between the two sets of styles', () => { var styles = {opacity: 0, color: 'red'}; var newStyles = {background: 'red'}; var flag = '*'; - var result = animationUtils.balanceAnimationStyles(styles, newStyles, flag); + var result = animationUtils.prepareFinalAnimationStyles(styles, newStyles, flag); expect(result).toEqual({opacity: flag, color: flag, background: 'red'}) }); it('should handle an empty set of styles', () => { var value = '*'; - expect(animationUtils.balanceAnimationStyles({}, {opacity: '0'}, value)).toEqual({ + expect(animationUtils.prepareFinalAnimationStyles({}, {opacity: '0'}, value)).toEqual({ opacity: '0' }); - expect(animationUtils.balanceAnimationStyles({opacity: '0'}, {}, value)).toEqual({ + expect(animationUtils.prepareFinalAnimationStyles({opacity: '0'}, {}, value)).toEqual({ opacity: value }); }); + + it('should set all AUTO styles to the null value', () => { + var styles = {opacity: 0}; + var newStyles = {color: '*', border: '*'}; + var flag = '*'; + var result = animationUtils.prepareFinalAnimationStyles(styles, newStyles, null); + expect(result).toEqual({opacity: null, color: null, border: null}) + }); + }); describe('balanceAnimationKeyframes', () => { @@ -91,12 +100,9 @@ export function main() { describe('clearStyles', () => { it('should set all the style values to "null"', () => { - var styles = {'opacity': 0, 'width': 100, 'color': 'red'}; - var expectedResult = { - 'opacity': null as number, - 'width': null as number, - 'color': null as string - }; + var styles: {[key: string]: string | number} = {'opacity': 0, 'width': 100, 'color': 'red'}; + var expectedResult: {[key: string]: + string | number} = {'opacity': null, 'width': null, 'color': null}; expect(animationUtils.clearStyles(styles)).toEqual(expectedResult); }); diff --git a/modules/@angular/platform-browser/core_private.ts b/modules/@angular/platform-browser/core_private.ts index 3c810217bb..8bf3932256 100644 --- a/modules/@angular/platform-browser/core_private.ts +++ b/modules/@angular/platform-browser/core_private.ts @@ -30,7 +30,8 @@ export type AnimationKeyframe = t.AnimationKeyframe; export var AnimationKeyframe: typeof t.AnimationKeyframe = r.AnimationKeyframe; export type AnimationStyles = t.AnimationStyles; export var AnimationStyles: typeof t.AnimationStyles = r.AnimationStyles; -export var balanceAnimationStyles: typeof t.balanceAnimationStyles = r.balanceAnimationStyles; +export var prepareFinalAnimationStyles: typeof t.prepareFinalAnimationStyles = + r.prepareFinalAnimationStyles; export var balanceAnimationKeyframes: typeof t.balanceAnimationKeyframes = r.balanceAnimationKeyframes; export var flattenStyles: typeof t.flattenStyles = r.flattenStyles;