From 70628112e85ceadd2e0aa243a659f871cbf29492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 17 Aug 2017 12:50:33 -0700 Subject: [PATCH] fix(animations): restore auto-style support for removed DOM nodes (#18787) PR Close #18787 --- .../src/render/transition_animation_engine.ts | 58 ++++++- .../animation/animation_integration_spec.ts | 84 ++++----- ...ns_with_web_animations_integration_spec.ts | 163 +++++++++++++++++- 3 files changed, 254 insertions(+), 51 deletions(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 95b1081172..5e2d6763d1 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -967,16 +967,39 @@ export class TransitionAnimationEngine { }); }); - // PRE STAGE: fill the ! styles - const preStylesMap = allPreStyleElements.size ? - cloakAndComputeStyles( - this.driver, enterNodesWithoutAnimations, allPreStyleElements, PRE_STYLE) : - new Map(); + // this is a special case for nodes that will be removed (either by) + // having their own leave animations or by being queried in a container + // that will be removed once a parent animation is complete. The idea + // here is that * styles must be identical to ! styles because of + // backwards compatibility (* is also filled in by default in many places). + // Otherwise * styles will return an empty value or auto since the element + // that is being getComputedStyle'd will not be visible (since * = destination) + const replaceNodes = allLeaveNodes.filter(node => { + return replacePostStylesAsPre(node, allPreStyleElements, allPostStyleElements); + }); // POST STAGE: fill the * styles - const postStylesMap = cloakAndComputeStyles( + const [postStylesMap, allLeaveQueriedNodes] = cloakAndComputeStyles( this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE); + allLeaveQueriedNodes.forEach(node => { + if (replacePostStylesAsPre(node, allPreStyleElements, allPostStyleElements)) { + replaceNodes.push(node); + } + }); + + // PRE STAGE: fill the ! styles + const [preStylesMap] = allPreStyleElements.size ? + cloakAndComputeStyles( + this.driver, enterNodesWithoutAnimations, allPreStyleElements, PRE_STYLE) : + [new Map()]; + + replaceNodes.forEach(node => { + const post = postStylesMap.get(node); + const pre = preStylesMap.get(node); + postStylesMap.set(node, { ...post, ...pre } as any); + }); + const rootPlayers: TransitionAnimationPlayer[] = []; const subPlayers: TransitionAnimationPlayer[] = []; queuedInstructions.forEach(entry => { @@ -1413,9 +1436,10 @@ function cloakElement(element: any, value?: string) { function cloakAndComputeStyles( driver: AnimationDriver, elements: any[], elementPropsMap: Map>, - defaultStyle: string): Map { + defaultStyle: string): [Map, any[]] { const cloakVals = elements.map(element => cloakElement(element)); const valuesMap = new Map(); + const failedElements: any[] = []; elementPropsMap.forEach((props: Set, element: any) => { const styles: ɵStyleData = {}; @@ -1426,13 +1450,14 @@ function cloakAndComputeStyles( // by a parent animation element being detached. if (!value || value.length == 0) { element[REMOVAL_FLAG] = NULL_REMOVED_QUERIED_STATE; + failedElements.push(element); } }); valuesMap.set(element, styles); }); elements.forEach((element, i) => cloakElement(element, cloakVals[i])); - return valuesMap; + return [valuesMap, failedElements]; } /* @@ -1539,3 +1564,20 @@ function objEquals(a: {[key: string]: any}, b: {[key: string]: any}): boolean { } return true; } + +function replacePostStylesAsPre( + element: any, allPreStyleElements: Map>, + allPostStyleElements: Map>): boolean { + const postEntry = allPostStyleElements.get(element); + if (!postEntry) return false; + + let preEntry = allPreStyleElements.get(element); + if (preEntry) { + postEntry.forEach(data => preEntry !.add(data)); + } else { + allPreStyleElements.set(element, postEntry); + } + + allPostStyleElements.delete(element); + return true; +} diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 9694c7fc59..b2a6643956 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -1087,59 +1087,61 @@ export function main() { .toBeTruthy(); }); - it('should animate removals of nodes to the `void` state for each animation trigger', () => { - @Component({ - selector: 'ani-cmp', - template: ` + it('should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles', + () => { + @Component({ + selector: 'ani-cmp', + template: `
`, - animations: [ - trigger('trig1', [transition('state => void', [animate(1000, style({opacity: 0}))])]), - trigger('trig2', [transition(':leave', [animate(1000, style({width: '0px'}))])]) - ] - }) - class Cmp { - public exp = true; - public exp2 = 'state'; - } + animations: [ + trigger( + 'trig1', [transition('state => void', [animate(1000, style({opacity: 0}))])]), + trigger('trig2', [transition(':leave', [animate(1000, style({width: '0px'}))])]) + ] + }) + class Cmp { + public exp = true; + public exp2 = 'state'; + } - TestBed.configureTestingModule({declarations: [Cmp]}); + 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(); - resetLog(); + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = true; + fixture.detectChanges(); + engine.flush(); + resetLog(); - const element = getDOM().querySelector(fixture.nativeElement, '.ng-if'); - assertHasParent(element, true); + const element = getDOM().querySelector(fixture.nativeElement, '.ng-if'); + assertHasParent(element, true); - cmp.exp = false; - fixture.detectChanges(); - engine.flush(); + cmp.exp = false; + fixture.detectChanges(); + engine.flush(); - assertHasParent(element, true); + assertHasParent(element, true); - expect(getLog().length).toEqual(2); + expect(getLog().length).toEqual(2); - const player2 = getLog().pop() !; - const player1 = getLog().pop() !; + const player2 = getLog().pop() !; + const player1 = getLog().pop() !; - expect(player2.keyframes).toEqual([ - {width: AUTO_STYLE, offset: 0}, - {width: '0px', offset: 1}, - ]); + expect(player2.keyframes).toEqual([ + {width: PRE_STYLE, offset: 0}, + {width: '0px', offset: 1}, + ]); - expect(player1.keyframes).toEqual([ - {opacity: AUTO_STYLE, offset: 0}, {opacity: '0', offset: 1} - ]); + expect(player1.keyframes).toEqual([ + {opacity: PRE_STYLE, offset: 0}, {opacity: '0', offset: 1} + ]); - player2.finish(); - player1.finish(); - assertHasParent(element, false); - }); + player2.finish(); + player1.finish(); + assertHasParent(element, false); + }); it('should properly cancel all existing animations when a removal occurs', () => { @Component({ 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 e257209fe0..aab246ed8e 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,8 +5,9 @@ * 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, query, state, style, transition, trigger} from '@angular/animations'; +import {animate, group, query, state, style, transition, trigger} from '@angular/animations'; import {AnimationDriver, ɵAnimationEngine, ɵWebAnimationsDriver, ɵWebAnimationsPlayer, ɵsupportsWebAnimations} from '@angular/animations/browser'; +import {TransitionAnimationPlayer} from '@angular/animations/browser/src/render/transition_animation_engine'; import {AnimationGroupPlayer} from '@angular/animations/src/players/animation_group_player'; import {Component} from '@angular/core'; import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; @@ -127,7 +128,7 @@ export function main() { template: `
- - {{ item }} + - {{ item }}
`, @@ -177,6 +178,164 @@ export function main() { ]); }); + it('should treat * styles as ! when a removal animation is being rendered', () => { + @Component({ + selector: 'ani-cmp', + styles: [` + .box { + width: 500px; + overflow:hidden; + background:orange; + line-height:300px; + font-size:100px; + text-align:center; + } + `], + template: ` + +
+
+ ... +
+ `, + animations: [trigger( + 'slide', + [ + state('void', style({height: '0px'})), + state('*', style({height: '*'})), + transition('* => *', animate('500ms')), + ])] + }) + class Cmp { + exp = false; + + toggle() { this.exp = !this.exp; } + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.exp = true; + fixture.detectChanges(); + + let player = engine.players[0] !; + let webPlayer = player.getRealPlayer() as ɵWebAnimationsPlayer; + expect(webPlayer.keyframes).toEqual([ + {height: '0px', offset: 0}, + {height: '300px', offset: 1}, + ]); + player.finish(); + + cmp.exp = false; + fixture.detectChanges(); + + player = engine.players[0] !; + webPlayer = player.getRealPlayer() as ɵWebAnimationsPlayer; + expect(webPlayer.keyframes).toEqual([ + {height: '300px', offset: 0}, + {height: '0px', offset: 1}, + ]); + }); + + it('should treat * styles as ! for queried items that are collected in a container that is being removed', + () => { + @Component({ + selector: 'my-app', + styles: [` + .list .outer { + overflow:hidden; + } + .list .inner { + line-height:50px; + } + `], + template: ` + + + +
+
+
+
+ {{ item }} +
+
+
+ `, + animations: [ + trigger('list', [ + transition(':enter', []), + transition('* => empty', [ + query(':leave', [ + animate(500, style({ height: '0px' })) + ]) + ]), + transition('* => full', [ + query(':enter', [ + style({ height: '0px' }), + animate(500, style({ height: '*' })) + ]) + ]), + ]) + ] + }) + class Cmp { + items: any[] = []; + + get exp() { return this.items.length ? 'full' : 'empty'; } + + empty() { this.items = []; } + + full() { this.items = [0, 1, 2, 3, 4]; } + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.empty(); + fixture.detectChanges(); + let player = engine.players[0] !as TransitionAnimationPlayer; + player.finish(); + + cmp.full(); + fixture.detectChanges(); + + player = engine.players[0] !as TransitionAnimationPlayer; + let queriedPlayers = (player.getRealPlayer() as AnimationGroupPlayer).players; + expect(queriedPlayers.length).toEqual(5); + + let i = 0; + for (i = 0; i < queriedPlayers.length; i++) { + let player = queriedPlayers[i] as ɵWebAnimationsPlayer; + expect(player.keyframes).toEqual([ + {height: '0px', offset: 0}, + {height: '50px', offset: 1}, + ]); + player.finish(); + } + + cmp.empty(); + fixture.detectChanges(); + + player = engine.players[0] !as TransitionAnimationPlayer; + queriedPlayers = (player.getRealPlayer() as AnimationGroupPlayer).players; + expect(queriedPlayers.length).toEqual(5); + + for (i = 0; i < queriedPlayers.length; i++) { + let player = queriedPlayers[i] as ɵWebAnimationsPlayer; + expect(player.keyframes).toEqual([ + {height: '50px', offset: 0}, + {height: '0px', offset: 1}, + ]); + } + }); + it('should compute intermediate styles properly when an animation is cancelled', () => { @Component({ selector: 'ani-cmp',