From fbccd5cd38bb76971211a0bd833e94188d9f75ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Mon, 20 Mar 2017 16:31:55 -0700 Subject: [PATCH] fix(animations): ensure empty animate() steps work at the end of a sequence (#15328) Closes #15310 Closes #15328 PR Close #15328 --- .../src/dsl/animation_timeline_visitor.ts | 104 ++++++++++-------- .../animation/animation_integration_spec.ts | 75 +++++++++++++ 2 files changed, 133 insertions(+), 46 deletions(-) diff --git a/packages/animations/browser/src/dsl/animation_timeline_visitor.ts b/packages/animations/browser/src/dsl/animation_timeline_visitor.ts index b8f5a82e63..a2d5f03d5c 100644 --- a/packages/animations/browser/src/dsl/animation_timeline_visitor.ts +++ b/packages/animations/browser/src/dsl/animation_timeline_visitor.ts @@ -150,34 +150,18 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor { context.currentTimeline.setStyles(startingStyles); visitAnimationNode(this, ast, context); - const normalizedFinalStyles = copyStyles(finalStyles, true); - // this is a special case for when animate(TIME) is used (without any styles) - // thus indicating to create an animation arc between the final keyframe and - // the destination styles. When this occurs we need to ensure that the styles - // that are missing on the finalStyles map are set to AUTO - if (Object.keys(context.currentTimeline.getFinalKeyframe()).length == 0) { - context.currentTimeline.properties.forEach(prop => { - const val = normalizedFinalStyles[prop]; - if (val == null) { - normalizedFinalStyles[prop] = AUTO_STYLE; - } - }); - } - - context.currentTimeline.setStyles(normalizedFinalStyles); - const timelineInstructions: AnimationTimelineInstruction[] = []; - context.timelines.forEach(timeline => { - // this checks to see if an actual animation happened - if (timeline.hasStyling()) { - timelineInstructions.push(timeline.buildKeyframes()); + // this checks to see if an actual animation happened + const timelines = context.timelines.filter(timeline => timeline.hasStyling()); + if (timelines.length && Object.keys(finalStyles).length) { + const tl = timelines[timelines.length - 1]; + if (!tl.allowOnlyTimelineStyles()) { + tl.setStyles(finalStyles); } - }); - - if (timelineInstructions.length == 0) { - timelineInstructions.push(createTimelineInstruction([], 0, 0, '')); } - return timelineInstructions; + + return timelines.length ? timelines.map(timeline => timeline.buildKeyframes()) : + [createTimelineInstruction([], 0, 0, '')]; } visitState(ast: AnimationStateMetadata, context: any): any { @@ -242,8 +226,13 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor { this.visitKeyframeSequence(ast.styles, context); } else { let styleAst = ast.styles as AnimationStyleMetadata; - if (!styleAst && timings.easing) { - styleAst = style({easing: timings.easing}); + if (!styleAst) { + const newStyleData: {[prop: string]: string | number} = {}; + if (timings.easing) { + newStyleData['easing'] = timings.easing; + } + styleAst = style(newStyleData); + (styleAst as any)['treatAsEmptyStep'] = true; } context.incrementTime(timings.duration); if (styleAst) { @@ -267,16 +256,19 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor { const normalizedStyles = normalizeStyles(ast.styles); const easing = context.currentAnimateTimings && context.currentAnimateTimings.easing; - this._applyStyles(normalizedStyles, easing, context); + this._applyStyles( + normalizedStyles, easing, (ast as any)['treatAsEmptyStep'] ? true : false, context); context.previousNode = ast; } - private _applyStyles(styles: ɵStyleData, easing: string, context: AnimationTimelineContext) { + private _applyStyles( + styles: ɵStyleData, easing: string, treatAsEmptyStep: boolean, + context: AnimationTimelineContext) { if (styles.hasOwnProperty('easing')) { easing = easing || styles['easing'] as string; delete styles['easing']; } - context.currentTimeline.setStyles(styles, easing); + context.currentTimeline.setStyles(styles, easing, treatAsEmptyStep); } visitKeyframeSequence( @@ -303,7 +295,7 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor { (step.offset != null ? step.offset : parseFloat(normalizedStyles['offset'] as string)) : (i == limit ? MAX_KEYFRAME_OFFSET : i * offsetGap); innerTimeline.forwardTime(offset * duration); - this._applyStyles(normalizedStyles, null, innerContext); + this._applyStyles(normalizedStyles, null, false, innerContext); }); // this will ensure that the parent timeline gets all the styles from @@ -320,12 +312,14 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor { export class TimelineBuilder { public duration: number = 0; public easing: string = ''; + private _previousKeyframe: ɵStyleData = {}; private _currentKeyframe: ɵStyleData; private _keyframes = new Map(); private _styleSummary: {[prop: string]: StyleAtTime} = {}; private _localTimelineStyles: ɵStyleData; private _backFill: ɵStyleData = {}; + private _currentEmptyStepKeyframe: ɵStyleData = null; constructor(public startTime: number, private _globalTimelineStyles: ɵStyleData = null) { this._localTimelineStyles = Object.create(this._backFill, {}); @@ -370,25 +364,43 @@ export class TimelineBuilder { this._styleSummary[prop] = {time: this.currentTime, value}; } - setStyles(styles: ɵStyleData, easing: string = null) { + allowOnlyTimelineStyles() { return this._currentEmptyStepKeyframe !== this._currentKeyframe; } + + setStyles(styles: ɵStyleData, easing: string = null, treatAsEmptyStep: boolean = false) { if (easing) { this._previousKeyframe['easing'] = easing; } - Object.keys(styles).forEach(prop => { - if (prop !== 'offset') { - const val = styles[prop]; - this._currentKeyframe[prop] = val; - if (!this._localTimelineStyles[prop]) { - this._backFill[prop] = this._globalTimelineStyles[prop] || AUTO_STYLE; + + if (treatAsEmptyStep) { + // special case for animate(duration): + // all missing styles are filled with a `*` value then + // if any destination styles are filled in later on the same + // keyframe then they will override the overridden styles + // We use `_globalTimelineStyles` here because there may be + // styles in previous keyframes that are not present in this timeline + Object.keys(this._globalTimelineStyles).forEach(prop => { + this._backFill[prop] = this._globalTimelineStyles[prop] || AUTO_STYLE; + this._currentKeyframe[prop] = AUTO_STYLE; + }); + this._currentEmptyStepKeyframe = this._currentKeyframe; + } else { + Object.keys(styles).forEach(prop => { + if (prop !== 'offset') { + const val = styles[prop]; + this._currentKeyframe[prop] = val; + if (!this._localTimelineStyles[prop]) { + this._backFill[prop] = this._globalTimelineStyles[prop] || AUTO_STYLE; + } + this._updateStyle(prop, val); } - this._updateStyle(prop, val); - } - }); - Object.keys(this._localTimelineStyles).forEach(prop => { - if (!this._currentKeyframe.hasOwnProperty(prop)) { - this._currentKeyframe[prop] = this._localTimelineStyles[prop]; - } - }); + }); + + Object.keys(this._localTimelineStyles).forEach(prop => { + if (!this._currentKeyframe.hasOwnProperty(prop)) { + this._currentKeyframe[prop] = this._localTimelineStyles[prop]; + } + }); + } } snapshotCurrentStyles() { copyStyles(this._localTimelineStyles, false, this._currentKeyframe); } diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 5aad80a08a..b2114c90de 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -508,6 +508,81 @@ export function main() { expect(p3.previousStyles).toEqual({}); }); + it('should properly balance styles between states even if there are no destination state styles', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ `, + animations: [trigger( + 'myAnimation', + [ + state('void', style({opacity: 0, width: '0px', height: '0px'})), + transition(':enter', animate(1000)) + ])] + }) + class Cmp { + exp: boolean = false; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = true; + + fixture.detectChanges(); + engine.flush(); + + const [p1] = getLog(); + expect(p1.keyframes).toEqual([ + {opacity: '0', width: '0px', height: '0px', offset: 0}, + {opacity: AUTO_STYLE, width: AUTO_STYLE, height: AUTO_STYLE, offset: 1} + ]); + }); + + it('should not apply the destination styles if the final animate step already contains styles', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ `, + animations: [trigger( + 'myAnimation', + [ + state('void', style({color: 'red'})), state('*', style({color: 'blue'})), + transition( + ':enter', + [style({fontSize: '0px '}), animate(1000, style({fontSize: '100px'}))]) + ])] + }) + class Cmp { + exp: boolean = false; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = true; + + fixture.detectChanges(); + engine.flush(); + + const players = getLog(); + expect(players.length).toEqual(1); + + // notice how the final color is NOT blue + expect(players[0].keyframes).toEqual([ + {fontSize: '0px', color: 'red', offset: 0}, + {fontSize: '100px', color: 'red', offset: 1} + ]); + }); + it('should invoke an animation trigger that is state-less', () => { @Component({ selector: 'ani-cmp',