From a2f351b2c94c1ac71d009e868d40f18fab486d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 23 Jun 2017 16:34:10 -0700 Subject: [PATCH] fix(animations): properly collect :enter nodes that exist within multi-level DOM trees Closes #17632 --- .../src/render/transition_animation_engine.ts | 123 ++++++------------ .../animation_query_integration_spec.ts | 85 ++++++++++++ 2 files changed, 123 insertions(+), 85 deletions(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 936678506d60f..29e5b43b48244 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -94,9 +94,6 @@ export const VOID_VALUE = 'void'; export const DEFAULT_STATE_VALUE = new StateValue(VOID_VALUE); export const DELETED_STATE_VALUE = new StateValue('DELETED'); -const POTENTIAL_ENTER_CLASSNAME = ENTER_CLASSNAME + '-temp'; -const POTENTIAL_ENTER_SELECTOR = '.' + POTENTIAL_ENTER_CLASSNAME; - export class AnimationTransitionNamespace { public players: TransitionAnimationPlayer[] = []; @@ -749,14 +746,18 @@ export class TransitionAnimationEngine { const allPreStyleElements = new Map>(); const allPostStyleElements = new Map>(); - // this must occur before the instructions are built below such that - // the :enter queries match the elements (since the timeline queries - // are fired during instruction building). const bodyNode = getBodyNode(); const allEnterNodes: any[] = this.collectedEnterElements.length ? - collectEnterElements(this.driver, this.collectedEnterElements) : + this.collectedEnterElements.filter(createIsRootFilterFn(this.collectedEnterElements)) : []; + // this must occur before the instructions are built below such that + // the :enter queries match the elements (since the timeline queries + // are fired during instruction building). + for (let i = 0; i < allEnterNodes.length; i++) { + addClass(allEnterNodes[i], ENTER_CLASSNAME); + } + const allLeaveNodes: any[] = []; const leaveNodesWithoutAnimations: any[] = []; for (let i = 0; i < this.collectedLeaveElements.length; i++) { @@ -1283,78 +1284,6 @@ function cloakElement(element: any, value?: string) { return oldValue; } -/* -1. start from the root, find the first matching child - a) if not found then check to see if a previously stopped node was set in the stack - -> if so then use that as the nextCursor - b) if no queried item and no parent then stop completely - c) if no queried item and yes parent then jump to the parent and restart loop -2. visit the next node, check if matches - a) if doesn't exist then set that as the cursor and repeat - -> add to the previous cursor stack when the inner queries return nothing - b) if matches then add it and continue - */ -function filterNodeClasses( - driver: AnimationDriver, rootElement: any | null, selector: string): any[] { - const rootElements: any[] = []; - if (!rootElement) return rootElements; - - let cursor: any = rootElement; - let nextCursor: any = {}; - let potentialCursorStack: any[] = []; - do { - // 1. query from root - nextCursor = cursor ? driver.query(cursor, selector, false)[0] : null; - - // this is used to avoid the extra matchesElement call when we - // know that the element does match based it on being queried - let justQueried = !!nextCursor; - - if (!nextCursor) { - const nextPotentialCursor = potentialCursorStack.pop(); - if (nextPotentialCursor) { - // 1a) - nextCursor = nextPotentialCursor; - } else { - cursor = cursor.parentElement; - // 1b) - if (!cursor) break; - // 1c) - nextCursor = cursor = cursor.nextElementSibling; - continue; - } - } - - // 2. visit the next node - while (nextCursor) { - const matches = justQueried || driver.matchesElement(nextCursor, selector); - justQueried = false; - - const nextPotentialCursor = nextCursor.nextElementSibling; - - // 2a) - if (!matches) { - potentialCursorStack.push(nextPotentialCursor); - cursor = nextCursor; - break; - } - - // 2b) - rootElements.push(nextCursor); - nextCursor = nextPotentialCursor; - if (nextCursor) { - cursor = nextCursor; - } else { - cursor = cursor.parentElement; - if (!cursor) break; - nextCursor = cursor = cursor.nextElementSibling; - } - } - } while (nextCursor && nextCursor !== rootElement); - - return rootElements; -} - function cloakAndComputeStyles( driver: AnimationDriver, elements: any[], elementPropsMap: Map>, defaultStyle: string): Map { @@ -1379,12 +1308,36 @@ function cloakAndComputeStyles( return valuesMap; } -function collectEnterElements(driver: AnimationDriver, allEnterNodes: any[]) { - allEnterNodes.forEach(element => addClass(element, POTENTIAL_ENTER_CLASSNAME)); - const enterNodes = filterNodeClasses(driver, getBodyNode(), POTENTIAL_ENTER_SELECTOR); - enterNodes.forEach(element => addClass(element, ENTER_CLASSNAME)); - allEnterNodes.forEach(element => removeClass(element, POTENTIAL_ENTER_CLASSNAME)); - return enterNodes; +/* +Since the Angular renderer code will return a collection of inserted +nodes in all areas of a DOM tree, it's up to this algorithm to figure +out which nodes are roots. + +By placing all nodes into a set and traversing upwards to the edge, +the recursive code can figure out if a clean path from the DOM node +to the edge container is clear. If no other node is detected in the +set then it is a root element. + +This algorithm also keeps track of all nodes along the path so that +if other sibling nodes are also tracked then the lookup process can +skip a lot of steps in between and avoid traversing the entire tree +multiple times to the edge. + */ +function createIsRootFilterFn(nodes: any): (node: any) => boolean { + const nodeSet = new Set(nodes); + const knownRootContainer = new Set(); + let isRoot: (node: any) => boolean; + isRoot = node => { + if (!node) return true; + if (nodeSet.has(node.parentNode)) return false; + if (knownRootContainer.has(node.parentNode)) return true; + if (isRoot(node.parentNode)) { + knownRootContainer.add(node); + return true; + } + return false; + }; + return isRoot; } const CLASSES_CACHE_KEY = '$$classes'; diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index 3bce03e2b411b..4044f868a6297 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -851,6 +851,91 @@ export function main() { expect(p4.element.innerText.trim()).toEqual('4'); }); + it('should find :enter/:leave nodes that are nested inside of ng-container elements', () => { + @Component({ + selector: 'ani-cmp', + template: ` +
+ +
+
even {{ item }}
+
odd {{ item }}
+
+ +
+ `, + animations: [trigger( + 'myAnimation', + [ + transition('0 => 5', [ + query(':enter', [ + style({ opacity: '0' }), + animate(1000, style({ opacity: '1' })) + ]) + ]), + transition('5 => 0', [ + query(':leave', [ + style({ opacity: '1' }), + animate(1000, style({ opacity: '0' })) + ]) + ]), + ])] + }) + class Cmp { + public items: any[]; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + + cmp.items = []; + fixture.detectChanges(); + engine.flush(); + resetLog(); + + cmp.items = [0, 1, 2, 3, 4]; + fixture.detectChanges(); + engine.flush(); + + let players = getLog(); + expect(players.length).toEqual(5); + + for (let i = 0; i < 5; i++) { + let player = players[i] !; + expect(player.keyframes).toEqual([ + {opacity: '0', offset: 0}, + {opacity: '1', offset: 1}, + ]); + + let elm = player.element; + let text = i % 2 == 0 ? `even ${i}` : `odd ${i}`; + expect(elm.innerText.trim()).toEqual(text); + } + + resetLog(); + cmp.items = []; + fixture.detectChanges(); + engine.flush(); + + players = getLog(); + expect(players.length).toEqual(5); + + for (let i = 0; i < 5; i++) { + let player = players[i] !; + expect(player.keyframes).toEqual([ + {opacity: '1', offset: 0}, + {opacity: '0', offset: 1}, + ]); + + let elm = player.element; + let text = i % 2 == 0 ? `even ${i}` : `odd ${i}`; + expect(elm.innerText.trim()).toEqual(text); + } + }); + it('should properly cancel items that were queried into a former animation', () => { @Component({ selector: 'ani-cmp',