diff --git a/packages/animations/browser/src/render/dom_animation_engine.ts b/packages/animations/browser/src/render/dom_animation_engine.ts index 4e6119b55f..5756c08384 100644 --- a/packages/animations/browser/src/render/dom_animation_engine.ts +++ b/packages/animations/browser/src/render/dom_animation_engine.ts @@ -217,9 +217,9 @@ export class DomAnimationEngine { // we first run this so that the previous animation player // data can be passed into the successive animation players let totalTime = 0; - const players = instruction.timelines.map(timelineInstruction => { + const players = instruction.timelines.map((timelineInstruction, i) => { totalTime = Math.max(totalTime, timelineInstruction.totalTime); - return this._buildPlayer(element, timelineInstruction, previousPlayers); + return this._buildPlayer(element, timelineInstruction, previousPlayers, i); }); previousPlayers.forEach(previousPlayer => previousPlayer.destroy()); @@ -253,8 +253,8 @@ export class DomAnimationEngine { public animateTimeline( element: any, instructions: AnimationTimelineInstruction[], previousPlayers: AnimationPlayer[] = []): AnimationPlayer { - const players = instructions.map(instruction => { - const player = this._buildPlayer(element, instruction, previousPlayers); + const players = instructions.map((instruction, i) => { + const player = this._buildPlayer(element, instruction, previousPlayers, i); player.onDestroy( () => { deleteFromArrayMap(this._activeElementAnimations, element, player); }); player.init(); @@ -266,8 +266,14 @@ export class DomAnimationEngine { } private _buildPlayer( - element: any, instruction: AnimationTimelineInstruction, - previousPlayers: AnimationPlayer[]): AnimationPlayer { + element: any, instruction: AnimationTimelineInstruction, previousPlayers: AnimationPlayer[], + index: number = 0): AnimationPlayer { + // only the very first animation can absorb the previous styles. This + // is here to prevent the an overlap situation where a group animation + // absorbs previous styles multiple times for the same element. + if (index && previousPlayers.length) { + previousPlayers = []; + } return this._driver.animate( element, this._normalizeKeyframes(instruction.keyframes), instruction.duration, instruction.delay, instruction.easing, previousPlayers); diff --git a/packages/animations/browser/src/render/web_animations/web_animations_player.ts b/packages/animations/browser/src/render/web_animations/web_animations_player.ts index 940c329ef7..d0368042e0 100644 --- a/packages/animations/browser/src/render/web_animations/web_animations_player.ts +++ b/packages/animations/browser/src/render/web_animations/web_animations_player.ts @@ -71,7 +71,7 @@ export class WebAnimationsPlayer implements AnimationPlayer { let startingKeyframe = keyframes[0]; let missingStyleProps: string[] = []; previousStyleProps.forEach(prop => { - if (startingKeyframe[prop] != null) { + if (!startingKeyframe.hasOwnProperty(prop)) { missingStyleProps.push(prop); } startingKeyframe[prop] = this.previousStyles[prop]; @@ -102,7 +102,7 @@ export class WebAnimationsPlayer implements AnimationPlayer { _triggerWebAnimation(element: any, keyframes: any[], options: any): DOMAnimation { // jscompiler doesn't seem to know animate is a native property because it's not fully // supported yet across common browsers (we polyfill it for Edge/Safari) [CL #143630929] - return element['animate'](keyframes, options); + return element['animate'](keyframes, options) as DOMAnimation; } get domPlayer() { return this._player; } diff --git a/packages/animations/browser/test/render/web_animations_player_spec.ts b/packages/animations/browser/test/render/web_animations_player_spec.ts new file mode 100644 index 0000000000..93c56f6c15 --- /dev/null +++ b/packages/animations/browser/test/render/web_animations_player_spec.ts @@ -0,0 +1,78 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {DOMAnimation} from '../../src/render/web_animations/dom_animation'; +import {WebAnimationsPlayer} from '../../src/render/web_animations/web_animations_player'; + +export function main() { + describe('WebAnimationsPlayer', function() { + // these tests are only mean't to be run within the DOM + if (typeof Element == 'undefined') return; + + let element: any; + beforeEach(() => { + element = document.createElement('div'); + document.body.appendChild(element); + }); + + afterEach(() => { document.body.removeChild(element); }); + + it('should properly balance any previous player styles into the animation keyframes', () => { + element.style.height = '666px'; + element.style.width = '333px'; + + const prevPlayer1 = new MockWebAnimationsPlayer( + element, [{width: '0px', offset: 0}, {width: '200px', offset: 1}], {}); + prevPlayer1.play(); + prevPlayer1.finish(); + + const prevPlayer2 = new MockWebAnimationsPlayer( + element, [{height: '0px', offset: 0}, {height: '200px', offset: 1}], {}); + prevPlayer2.play(); + prevPlayer2.finish(); + + // what needs to happen here is the player below should + // examine which styles are present in the provided previous + // players and use them as input data for the keyframes of + // the new player. Given that the players are in their finished + // state, the styles are copied over as the starting keyframe + // for the animation and if the styles are missing in later keyframes + // then the styling is resolved by computing the styles + const player = new MockWebAnimationsPlayer( + element, [{width: '100px', offset: 0}, {width: '500px', offset: 1}], {}, + [prevPlayer1, prevPlayer2]); + + player.init(); + expect(player.capturedKeyframes).toEqual([ + {height: '200px', width: '200px', offset: 0}, + {height: '666px', width: '500px', offset: 1} + ]); + }); + }); +} + +class MockWebAnimationsPlayer extends WebAnimationsPlayer { + capturedKeyframes: any[]; + + _triggerWebAnimation(element: any, keyframes: any[], options: any): any { + this.capturedKeyframes = keyframes; + return new MockDOMAnimation(); + } +} + +class MockDOMAnimation implements DOMAnimation { + onfinish = (callback: (e: any) => any) => {}; + position = 0; + currentTime = 0; + + cancel(): void {} + play(): void {} + pause(): void {} + finish(): void {} + addEventListener(eventName: string, handler: (event: any) => any): any { return null; } + dispatchEvent(eventName: string): any { return null; } +} diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index ed6ef269c2..05cf9f74c7 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {AUTO_STYLE, AnimationEvent, animate, keyframes, state, style, transition, trigger} from '@angular/animations'; +import {AUTO_STYLE, AnimationEvent, animate, group, keyframes, state, style, transition, trigger} from '@angular/animations'; import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver} from '@angular/animations/browser'; import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing'; import {Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core'; @@ -404,8 +404,9 @@ export function main() { })); }); - it('should cancel and merge in mid-animation styles into the follow-up animation', () => { - @Component({ + it('should cancel and merge in mid-animation styles into the follow-up animation, but only for animation keyframes that start right away', + () => { + @Component({ selector: 'ani-cmp', template: `
@@ -422,41 +423,50 @@ export function main() { transition( 'b => c', [ - style({'width': '0'}), - animate(500, style({'width': '100px'})), + group([ + animate(500, style({'width': '100px'})), + animate(500, style({'height': '100px'})), + ]), + animate(500, keyframes([ + style({'opacity': '0'}), + style({'opacity': '1'}) + ])) ]), ])], }) class Cmp { - exp: any = false; - } + exp: any = false; + } - TestBed.configureTestingModule({declarations: [Cmp]}); + TestBed.configureTestingModule({declarations: [Cmp]}); - const engine = TestBed.get(ɵAnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; - cmp.exp = 'a'; - fixture.detectChanges(); - engine.flush(); - expect(getLog().length).toEqual(0); - resetLog(); + cmp.exp = 'a'; + fixture.detectChanges(); + engine.flush(); + expect(getLog().length).toEqual(0); + resetLog(); - cmp.exp = 'b'; - fixture.detectChanges(); - engine.flush(); - expect(getLog().length).toEqual(1); - resetLog(); + cmp.exp = 'b'; + fixture.detectChanges(); + engine.flush(); + expect(getLog().length).toEqual(1); + resetLog(); - cmp.exp = 'c'; - fixture.detectChanges(); - engine.flush(); - expect(getLog().length).toEqual(1); + cmp.exp = 'c'; + fixture.detectChanges(); + engine.flush(); - const data = getLog().pop(); - expect(data.previousStyles).toEqual({opacity: AUTO_STYLE}); - }); + const players = getLog(); + expect(players.length).toEqual(3); + const [p1, p2, p3] = players; + expect(p1.previousStyles).toEqual({opacity: AUTO_STYLE}); + expect(p2.previousStyles).toEqual({}); + expect(p3.previousStyles).toEqual({}); + }); it('should invoke an animation trigger that is state-less', () => { @Component({