From 185075d87098f08e12edd5401628dafb1ae34e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 9 Jun 2017 14:39:36 -0700 Subject: [PATCH] fix(animations): compute removal node height correctly --- .../src/render/transition_animation_engine.ts | 43 ++++++--- ...ns_with_web_animations_integration_spec.ts | 93 ++++++++++++++++++- 2 files changed, 120 insertions(+), 16 deletions(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 5423f37783..bbe6ca3e48 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -740,18 +740,20 @@ export class TransitionAnimationEngine { // the :enter queries match the elements (since the timeline queries // are fired during instruction building). const bodyNode = getBodyNode(); - const allEnterNodes: any[] = this.collectedEnterElements; - const enterNodes: any[] = - allEnterNodes.length ? collectEnterElements(this.driver, allEnterNodes) : []; + const allEnterNodes: any[] = this.collectedEnterElements.length ? + collectEnterElements(this.driver, this.collectedEnterElements) : + []; - const leaveNodes: any[] = []; + const allLeaveNodes: any[] = []; + const leaveNodesWithoutAnimations: any[] = []; for (let i = 0; i < this.collectedLeaveElements.length; i++) { const element = this.collectedLeaveElements[i]; - if (isElementNode(element)) { - const details = element[REMOVAL_FLAG] as ElementAnimationState; - if (details && details.setForRemoval) { - addClass(element, LEAVE_CLASSNAME); - leaveNodes.push(element); + const details = element[REMOVAL_FLAG] as ElementAnimationState; + if (details && details.setForRemoval) { + addClass(element, LEAVE_CLASSNAME); + allLeaveNodes.push(element); + if (!details.hasAnimation) { + leaveNodesWithoutAnimations.push(element); } } } @@ -817,6 +819,16 @@ export class TransitionAnimationEngine { }); } + // these can only be detected here since we have a map of all the elements + // that have animations attached to them... + const enterNodesWithoutAnimations: any[] = []; + for (let i = 0; i < allEnterNodes.length; i++) { + const element = allEnterNodes[i]; + if (!subTimelines.has(element)) { + enterNodesWithoutAnimations.push(element); + } + } + const allPreviousPlayersMap = new Map(); let sortedParentElements: any[] = []; queuedInstructions.forEach(entry => { @@ -840,12 +852,13 @@ export class TransitionAnimationEngine { // PRE STAGE: fill the ! styles const preStylesMap = allPreStyleElements.size ? - cloakAndComputeStyles(this.driver, enterNodes, allPreStyleElements, PRE_STYLE) : + cloakAndComputeStyles( + this.driver, enterNodesWithoutAnimations, allPreStyleElements, PRE_STYLE) : new Map(); // POST STAGE: fill the * styles - const postStylesMap = - cloakAndComputeStyles(this.driver, leaveNodes, allPostStyleElements, AUTO_STYLE); + const postStylesMap = cloakAndComputeStyles( + this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE); const rootPlayers: TransitionAnimationPlayer[] = []; const subPlayers: TransitionAnimationPlayer[] = []; @@ -907,8 +920,8 @@ export class TransitionAnimationEngine { // run through all of the queued removals and see if they // were picked up by a query. If not then perform the removal // operation right away unless a parent animation is ongoing. - for (let i = 0; i < leaveNodes.length; i++) { - const element = leaveNodes[i]; + for (let i = 0; i < allLeaveNodes.length; i++) { + const element = allLeaveNodes[i]; const players = queriedElements.get(element); if (players) { removeNodesAfterAnimationDone(this, element, players); @@ -931,7 +944,7 @@ export class TransitionAnimationEngine { player.play(); }); - enterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME)); + allEnterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME)); return rootPlayers; } diff --git a/packages/core/test/animation/animations_with_web_animations_integration_spec.ts b/packages/core/test/animation/animations_with_web_animations_integration_spec.ts index 35be818182..1e03a142dd 100644 --- a/packages/core/test/animation/animations_with_web_animations_integration_spec.ts +++ b/packages/core/test/animation/animations_with_web_animations_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 {animate, style, transition, trigger} from '@angular/animations'; +import {animate, state, style, transition, trigger} from '@angular/animations'; import {AnimationDriver, ɵAnimationEngine} from '@angular/animations/browser'; import {ɵWebAnimationsDriver, ɵWebAnimationsPlayer, ɵsupportsWebAnimations} from '@angular/animations/browser' import {Component, ViewChild} from '@angular/core'; @@ -26,6 +26,97 @@ export function main() { }); }); + it('should compute (*) animation styles for a container that is being removed', () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
1
+
2
+
3
+
4
+
5
+
+ `, + animations: [trigger( + 'auto', + [ + state('void', style({height: '0px'})), state('*', style({height: '*'})), + transition('* => *', animate(1000)) + ])] + }) + class Cmp { + public 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(); + + expect(engine.players.length).toEqual(1); + let webPlayer = engine.players[0].getRealPlayer() as ɵWebAnimationsPlayer; + + expect(webPlayer.keyframes).toEqual([ + {height: '0px', offset: 0}, {height: '100px', offset: 1} + ]); + + cmp.exp = false; + fixture.detectChanges(); + engine.flush(); + + expect(engine.players.length).toEqual(1); + webPlayer = engine.players[0].getRealPlayer() as ɵWebAnimationsPlayer; + + expect(webPlayer.keyframes).toEqual([ + {height: '100px', offset: 0}, {height: '0px', offset: 1} + ]); + }); + + it('should compute (!) animation styles for a container that is being inserted', () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
1
+
2
+
3
+
4
+
5
+
+ `, + animations: [trigger( + 'auto', + [transition( + ':enter', [style({height: '!'}), animate(1000, style({height: '120px'}))])])] + }) + class Cmp { + public 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(); + + expect(engine.players.length).toEqual(1); + let webPlayer = engine.players[0].getRealPlayer() as ɵWebAnimationsPlayer; + + expect(webPlayer.keyframes).toEqual([ + {height: '100px', offset: 0}, {height: '120px', offset: 1} + ]); + }); + it('should compute pre (!) and post (*) animation styles with different dom states', () => { @Component({ selector: 'ani-cmp',