From d699c354dbff1958ff0fb19dedffd61b741323f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Mon, 26 Jun 2017 17:49:53 -0700 Subject: [PATCH] fix(animations): do not remove container nodes when children are queried by a parent animation Closes #17746 --- .../src/render/transition_animation_engine.ts | 54 ++++-- .../animation_query_integration_spec.ts | 158 ++++++++++++++++++ 2 files changed, 202 insertions(+), 10 deletions(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 7cafcd3722..936678506d 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -13,7 +13,7 @@ import {AnimationTransitionInstruction} from '../dsl/animation_transition_instru import {AnimationTrigger} from '../dsl/animation_trigger'; import {ElementInstructionMap} from '../dsl/element_instruction_map'; import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_style_normalizer'; -import {ENTER_CLASSNAME, LEAVE_CLASSNAME, NG_ANIMATING_CLASSNAME, NG_TRIGGER_CLASSNAME, NG_TRIGGER_SELECTOR, copyObj, eraseStyles, setStyles} from '../util'; +import {ENTER_CLASSNAME, LEAVE_CLASSNAME, NG_ANIMATING_CLASSNAME, NG_ANIMATING_SELECTOR, NG_TRIGGER_CLASSNAME, NG_TRIGGER_SELECTOR, copyObj, eraseStyles, setStyles} from '../util'; import {AnimationDriver} from './animation_driver'; import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared'; @@ -642,7 +642,8 @@ export class TransitionAnimationEngine { } destroyInnerAnimations(containerElement: any) { - this.driver.query(containerElement, NG_TRIGGER_SELECTOR, true).forEach(element => { + let elements = this.driver.query(containerElement, NG_TRIGGER_SELECTOR, true); + elements.forEach(element => { const players = this.playersByElement.get(element); if (players) { players.forEach(player => { @@ -661,6 +662,18 @@ export class TransitionAnimationEngine { Object.keys(stateMap).forEach(triggerName => stateMap[triggerName] = DELETED_STATE_VALUE); } }); + + if (this.playersByQueriedElement.size == 0) return; + + elements = this.driver.query(containerElement, NG_ANIMATING_SELECTOR, true); + if (elements.length) { + elements.forEach(element => { + const players = this.playersByQueriedElement.get(element); + if (players) { + players.forEach(player => player.finish()); + } + }); + } } whenRenderingDone(): Promise { @@ -922,14 +935,36 @@ export class TransitionAnimationEngine { // operation right away unless a parent animation is ongoing. for (let i = 0; i < allLeaveNodes.length; i++) { const element = allLeaveNodes[i]; - const players = queriedElements.get(element); - if (players) { + const details = element[REMOVAL_FLAG] as ElementAnimationState; + + // this means the element has a removal animation that is being + // taken care of and therefore the inner elements will hang around + // until that animation is over (or the parent queried animation) + if (details && details.hasAnimation) continue; + + let players: AnimationPlayer[] = []; + + // if this element is queried or if it contains queried children + // then we want for the element not to be removed from the page + // until the queried animations have finished + if (queriedElements.size) { + let queriedPlayerResults = queriedElements.get(element); + if (queriedPlayerResults && queriedPlayerResults.length) { + players.push(...queriedPlayerResults); + } + + let queriedInnerElements = this.driver.query(element, NG_ANIMATING_SELECTOR, true); + for (let j = 0; j < queriedInnerElements.length; j++) { + let queriedPlayers = queriedElements.get(queriedInnerElements[j]); + if (queriedPlayers && queriedPlayers.length) { + players.push(...queriedPlayers); + } + } + } + if (players.length) { removeNodesAfterAnimationDone(this, element, players); } else { - const details = element[REMOVAL_FLAG] as ElementAnimationState; - if (details && !details.hasAnimation) { - this.processLeaveNode(element); - } + this.processLeaveNode(element); } } @@ -1079,8 +1114,7 @@ export class TransitionAnimationEngine { allQueriedPlayers.forEach(player => { getOrSetAsInMap(this.playersByQueriedElement, player.element, []).push(player); - player.onDone( - () => { deleteOrUnsetInMap(this.playersByQueriedElement, player.element, player); }); + player.onDone(() => deleteOrUnsetInMap(this.playersByQueriedElement, player.element, player)); }); allConsumedElements.forEach(element => addClass(element, NG_ANIMATING_CLASSNAME)); diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index 8d591df3fb..3bce03e2b4 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -915,6 +915,164 @@ export function main() { expect(p4.previousPlayers).toEqual([]); }); + it('should not remove a parent container if its contents are queried into by an ancestor element', + () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
+
+
+
+
+ `, + animations: [ + trigger( + 'myAnimation', + [ + transition( + '* => go', + [ + query( + '.child', + [ + style({opacity: 0}), + animate(1000, style({opacity: 1})), + ]), + ]), + ]), + ] + }) + class Cmp { + public exp1: any = ''; + public exp2: any = true; + + @ViewChild('ancestor') public ancestorElm: any; + + @ViewChild('parent') public parentElm: any; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + engine.flush(); + resetLog(); + + const ancestorElm = cmp.ancestorElm.nativeElement; + const parentElm = cmp.parentElm.nativeElement; + + cmp.exp1 = 'go'; + cmp.exp2 = false; + fixture.detectChanges(); + engine.flush(); + + expect(ancestorElm.contains(parentElm)).toBe(true); + + const players = getLog(); + expect(players.length).toEqual(2); + const [p1, p2] = players; + expect(parentElm.contains(p1.element)).toBe(true); + expect(parentElm.contains(p2.element)).toBe(true); + + cancelAllPlayers(players); + + expect(ancestorElm.contains(parentElm)).toBe(false); + }); + + it('should only retain a to-be-removed node if the inner queried items are apart of an animation issued by an ancestor', + fakeAsync(() => { + @Component({ + selector: 'ani-cmp', + template: ` +
+
hello
+
+
child
+
+
+ `, + animations: [ + trigger( + 'one', + [ + transition( + '* => go', + [ + query( + '.child', + [ + style({height: '100px'}), + animate(1000, style({height: '0px'})), + ]), + ]), + ]), + trigger( + 'two', + [ + transition('* => go', [query( + 'header', + [ + style({width: '100px'}), + animate(1000, style({width: '0px'})), + ])]), + ]), + ] + }) + class Cmp { + public exp1: any = ''; + public exp2: any = ''; + public parentExp: any = true; + + @ViewChild('ancestor') public ancestorElm: any; + + @ViewChild('parent') public parentElm: any; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + engine.flush(); + resetLog(); + + const ancestorElm = cmp.ancestorElm.nativeElement; + const parentElm = cmp.parentElm.nativeElement; + expect(ancestorElm.contains(parentElm)).toBe(true); + + cmp.exp1 = 'go'; + fixture.detectChanges(); + engine.flush(); + + expect(ancestorElm.contains(parentElm)).toBe(true); + + const onePlayers = getLog(); + expect(onePlayers.length).toEqual(1); // element.child + const [childPlayer] = onePlayers; + + let childPlayerComplete = false; + childPlayer.onDone(() => childPlayerComplete = true); + resetLog(); + flushMicrotasks(); + + expect(childPlayerComplete).toBe(false); + + cmp.exp2 = 'go'; + cmp.parentExp = false; + fixture.detectChanges(); + engine.flush(); + + const twoPlayers = getLog(); + expect(twoPlayers.length).toEqual(1); // the header element + expect(ancestorElm.contains(parentElm)).toBe(false); + expect(childPlayerComplete).toBe(true); + })); + it('should finish queried players in an animation when the next animation takes over', () => { @Component({ selector: 'ani-cmp',