fix(animations): ensure multi-level enter animations work (angular#19455

)

PR Close angular#19455

packages/animations/browser/src/dsl/animation.ts

@@ -8,7 +8,7 @@
 import {AnimationMetadata, AnimationMetadataType, AnimationOptions, ɵStyleData} from '@angular/animations';
 
 import {AnimationDriver} from '../render/animation_driver';
-import {normalizeStyles} from '../util';
+import {ENTER_CLASSNAME, normalizeStyles} from '../util';
 
 import {Ast} from './animation_ast';
 import {buildAnimationAst} from './animation_ast_builder';
@@ -39,7 +39,8 @@ export class Animation {
     const errors: any = [];
     subInstructions = subInstructions || new ElementInstructionMap();
     const result = buildAnimationTimelines(
-        this._driver, element, this._animationAst, start, dest, options, subInstructions, errors);
+        this._driver, element, this._animationAst, ENTER_CLASSNAME, start, dest, options,
+        subInstructions, errors);
     if (errors.length) {
       const errorMessage = `animation building failed:\n${errors.join("\n")}`;
       throw new Error(errorMessage);

packages/animations/browser/src/dsl/animation_ast_builder.ts

@@ -62,8 +62,6 @@ export function buildAnimationAst(
 
 const LEAVE_TOKEN = ':leave';
 const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');
-const ENTER_TOKEN = ':enter';
-const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
 const ROOT_SELECTOR = '';
 
 export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
@@ -478,8 +476,7 @@ function normalizeSelector(selector: string): [string, boolean] {
     selector = selector.replace(SELF_TOKEN_REGEX, '');
   }
 
-  selector = selector.replace(ENTER_TOKEN_REGEX, ENTER_SELECTOR)
-                 .replace(LEAVE_TOKEN_REGEX, LEAVE_SELECTOR)
+  selector = selector.replace(LEAVE_TOKEN_REGEX, LEAVE_SELECTOR)
                  .replace(/@\*/g, NG_TRIGGER_SELECTOR)
                  .replace(/@\w+/g, match => NG_TRIGGER_SELECTOR + '-' + match.substr(1))
                  .replace(/:animating/g, NG_ANIMATING_SELECTOR);

packages/animations/browser/src/dsl/animation_timeline_builder.ts

@@ -15,6 +15,8 @@ import {AnimationTimelineInstruction, createTimelineInstruction} from './animati
 import {ElementInstructionMap} from './element_instruction_map';
 
 const ONE_FRAME_IN_MILLISECONDS = 1;
+const ENTER_TOKEN = ':enter';
+const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
 
 /*
  * The code within this file aims to generate web-animations-compatible keyframes from Angular's
@@ -101,20 +103,22 @@ const ONE_FRAME_IN_MILLISECONDS = 1;
  * the `AnimationValidatorVisitor` code.
  */
 export function buildAnimationTimelines(
-    driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
+    driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
     startingStyles: ɵStyleData = {}, finalStyles: ɵStyleData = {}, options: AnimationOptions,
     subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
   return new AnimationTimelineBuilderVisitor().buildKeyframes(
-      driver, rootElement, ast, startingStyles, finalStyles, options, subInstructions, errors);
+      driver, rootElement, ast, enterClassName, startingStyles, finalStyles, options,
+      subInstructions, errors);
 }
 
 export class AnimationTimelineBuilderVisitor implements AstVisitor {
   buildKeyframes(
-      driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
+      driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
       startingStyles: ɵStyleData, finalStyles: ɵStyleData, options: AnimationOptions,
       subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
     subInstructions = subInstructions || new ElementInstructionMap();
-    const context = new AnimationTimelineContext(driver, rootElement, subInstructions, errors, []);
+    const context = new AnimationTimelineContext(
+        driver, rootElement, subInstructions, enterClassName, errors, []);
     context.options = options;
     context.currentTimeline.setStyles([startingStyles], null, context.errors, options);
 
@@ -445,8 +449,9 @@ export class AnimationTimelineContext {
 
   constructor(
       private _driver: AnimationDriver, public element: any,
-      public subInstructions: ElementInstructionMap, public errors: any[],
-      public timelines: TimelineBuilder[], initialTimeline?: TimelineBuilder) {
+      public subInstructions: ElementInstructionMap, private _enterClassName: string,
+      public errors: any[], public timelines: TimelineBuilder[],
+      initialTimeline?: TimelineBuilder) {
     this.currentTimeline = initialTimeline || new TimelineBuilder(this._driver, element, 0);
     timelines.push(this.currentTimeline);
   }
@@ -499,8 +504,8 @@ export class AnimationTimelineContext {
       AnimationTimelineContext {
     const target = element || this.element;
     const context = new AnimationTimelineContext(
-        this._driver, target, this.subInstructions, this.errors, this.timelines,
-        this.currentTimeline.fork(target, newTime || 0));
+        this._driver, target, this.subInstructions, this._enterClassName, this.errors,
+        this.timelines, this.currentTimeline.fork(target, newTime || 0));
     context.previousNode = this.previousNode;
     context.currentAnimateTimings = this.currentAnimateTimings;
 
@@ -555,6 +560,7 @@ export class AnimationTimelineContext {
       results.push(this.element);
     }
     if (selector.length > 0) {  // if :self is only used then the selector is empty
+      selector = selector.replace(ENTER_TOKEN_REGEX, '.' + this._enterClassName);
       const multi = limit != 1;
       let elements = this._driver.query(this.element, selector, multi);
       if (limit !== 0) {

packages/animations/browser/src/dsl/animation_transition_factory.ts

@@ -37,7 +37,7 @@ export class AnimationTransitionFactory {
 
   build(
       driver: AnimationDriver, element: any, currentState: any, nextState: any,
-      currentOptions?: AnimationOptions, nextOptions?: AnimationOptions,
+      enterClassName: string, currentOptions?: AnimationOptions, nextOptions?: AnimationOptions,
       subInstructions?: ElementInstructionMap): AnimationTransitionInstruction {
     const errors: any[] = [];
 
@@ -55,8 +55,8 @@ export class AnimationTransitionFactory {
     const animationOptions = {params: {...transitionAnimationParams, ...nextAnimationParams}};
 
     const timelines = buildAnimationTimelines(
-        driver, element, this.ast.animation, currentStateStyles, nextStateStyles, animationOptions,
-        subInstructions, errors);
+        driver, element, this.ast.animation, enterClassName, currentStateStyles, nextStateStyles,
+        animationOptions, subInstructions, errors);
 
     if (errors.length) {
       return createTransitionInstruction(

packages/animations/browser/src/render/timeline_animation_engine.ts

@@ -13,6 +13,7 @@ import {buildAnimationTimelines} from '../dsl/animation_timeline_builder';
 import {AnimationTimelineInstruction} from '../dsl/animation_timeline_instruction';
 import {ElementInstructionMap} from '../dsl/element_instruction_map';
 import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_style_normalizer';
+import {ENTER_CLASSNAME} from '../util';
 
 import {AnimationDriver} from './animation_driver';
 import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
@@ -55,7 +56,8 @@ export class TimelineAnimationEngine {
 
     if (ast) {
       instructions = buildAnimationTimelines(
-          this._driver, element, ast, {}, {}, options, EMPTY_INSTRUCTION_MAP, errors);
+          this._driver, element, ast, ENTER_CLASSNAME, {}, {}, options, EMPTY_INSTRUCTION_MAP,
+          errors);
       instructions.forEach(inst => {
         const styles = getOrSetAsInMap(autoStylesMap, inst.element, {});
         inst.postStyleProps.forEach(prop => styles[prop] = null);

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_ANIMATING_SELECTOR, 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, iteratorToArray, setStyles} from '../util';
 
 import {AnimationDriver} from './animation_driver';
 import {getBodyNode, getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
@@ -714,9 +714,10 @@ export class TransitionAnimationEngine {
     return () => {};
   }
 
-  private _buildInstruction(entry: QueueInstruction, subTimelines: ElementInstructionMap) {
+  private _buildInstruction(
+      entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string) {
     return entry.transition.build(
-        this.driver, entry.element, entry.fromState.value, entry.toState.value,
+        this.driver, entry.element, entry.fromState.value, entry.toState.value, enterClassName,
         entry.fromState.options, entry.toState.options, subTimelines);
   }
 
@@ -862,16 +863,19 @@ export class TransitionAnimationEngine {
     });
 
     const bodyNode = getBodyNode();
-    const allEnterNodes: any[] = this.collectedEnterElements.length ?
-        this.collectedEnterElements.filter(createIsRootFilterFn(this.collectedEnterElements)) :
-        [];
+    const enterNodeMap =
+        buildRootMap(Array.from(this.statesByElement.keys()), 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 enterNodeMapIds = new Map<any, string>();
+    let i = 0;
+    enterNodeMap.forEach((nodes, root) => {
+      const className = ENTER_CLASSNAME + i++;
+      enterNodeMapIds.set(root, className);
+      nodes.forEach(node => addClass(node, className));
+    });
 
     const allLeaveNodes: any[] = [];
     const leaveNodesWithoutAnimations = new Set<any>();
@@ -888,7 +892,11 @@ export class TransitionAnimationEngine {
     }
 
     cleanupFns.push(() => {
-      allEnterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME));
+      enterNodeMap.forEach((nodes, root) => {
+        const className = enterNodeMapIds.get(root) !;
+        nodes.forEach(node => removeClass(node, className));
+      });
+
       allLeaveNodes.forEach(element => {
         removeClass(element, LEAVE_CLASSNAME);
         this.processLeaveNode(element);
@@ -909,7 +917,8 @@ export class TransitionAnimationEngine {
           return;
         }
 
-        const instruction = this._buildInstruction(entry, subTimelines) !;
+        const enterClassName = enterNodeMapIds.get(element) !;
+        const instruction = this._buildInstruction(entry, subTimelines, enterClassName) !;
         if (instruction.errors && instruction.errors.length) {
           erroneousTransitions.push(instruction);
           return;
@@ -973,18 +982,6 @@ export class TransitionAnimationEngine {
       this.reportError(errors);
     }
 
-    // these can only be detected here since we have a map of all the elements
-    // that have animations attached to them... We use a set here in the event
-    // multiple enter captures on the same element were caught in different
-    // renderer namespaces (e.g. when a @trigger was on a host binding that had *ngIf)
-    const enterNodesWithoutAnimations = new Set<any>();
-    for (let i = 0; i < allEnterNodes.length; i++) {
-      const element = allEnterNodes[i];
-      if (!subTimelines.has(element)) {
-        enterNodesWithoutAnimations.add(element);
-      }
-    }
-
     const allPreviousPlayersMap = new Map<any, TransitionAnimationPlayer[]>();
     // this map works to tell which element in the DOM tree is contained by
     // which animation. Further down below this map will get populated once
@@ -1022,8 +1019,9 @@ export class TransitionAnimationEngine {
     });
 
     // POST STAGE: fill the * styles
-    const [postStylesMap, allLeaveQueriedNodes] = cloakAndComputeStyles(
-        this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE);
+    const postStylesMap = new Map<any, ɵStyleData>();
+    const allLeaveQueriedNodes = cloakAndComputeStyles(
+        postStylesMap, this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE);
 
     allLeaveQueriedNodes.forEach(node => {
       if (replacePostStylesAsPre(node, allPreStyleElements, allPostStyleElements)) {
@@ -1032,10 +1030,11 @@ export class TransitionAnimationEngine {
     });
 
     // PRE STAGE: fill the ! styles
-    const [preStylesMap] = allPreStyleElements.size ?
-        cloakAndComputeStyles(
-            this.driver, enterNodesWithoutAnimations, allPreStyleElements, PRE_STYLE) :
-        [new Map<any, ɵStyleData>()];
+    const preStylesMap = new Map<any, ɵStyleData>();
+    enterNodeMap.forEach((nodes, root) => {
+      cloakAndComputeStyles(
+          preStylesMap, this.driver, new Set(nodes), allPreStyleElements, PRE_STYLE);
+    });
 
     replaceNodes.forEach(node => {
       const post = postStylesMap.get(node);
@@ -1505,12 +1504,11 @@ function cloakElement(element: any, value?: string) {
 }
 
 function cloakAndComputeStyles(
-    driver: AnimationDriver, elements: Set<any>, elementPropsMap: Map<any, Set<string>>,
-    defaultStyle: string): [Map<any, ɵStyleData>, any[]] {
+    valuesMap: Map<any, ɵStyleData>, driver: AnimationDriver, elements: Set<any>,
+    elementPropsMap: Map<any, Set<string>>, defaultStyle: string): any[] {
   const cloakVals: string[] = [];
   elements.forEach(element => cloakVals.push(cloakElement(element)));
 
-  const valuesMap = new Map<any, ɵStyleData>();
   const failedElements: any[] = [];
 
   elementPropsMap.forEach((props: Set<string>, element: any) => {
@@ -1532,39 +1530,57 @@ function cloakAndComputeStyles(
   // an index value for the closure (but instead just the value)
   let i = 0;
   elements.forEach(element => cloakElement(element, cloakVals[i++]));
-  return [valuesMap, failedElements];
+
+  return failedElements;
 }
 
 /*
 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.
+out which nodes are roots for each animation @trigger.
 
-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.
+By placing each inserted node into a Set and traversing upwards, it
+is possible to find the @trigger elements and well any direct *star
+insertion nodes, if a @trigger root is found then the enter element
+is placed into the Map[@trigger] spot.
  */
-function createIsRootFilterFn(nodes: any): (node: any) => boolean {
+function buildRootMap(roots: any[], nodes: any[]): Map<any, any[]> {
+  const rootMap = new Map<any, any[]>();
+  roots.forEach(root => rootMap.set(root, []));
+
+  if (nodes.length == 0) return rootMap;
+
+  const NULL_NODE = 1;
   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;
+  const localRootMap = new Map<any, any>();
+
+  function getRoot(node: any): any {
+    if (!node) return NULL_NODE;
+
+    let root = localRootMap.get(node);
+    if (root) return root;
+
+    const parent = node.parentNode;
+    if (rootMap.has(parent)) {  // ngIf inside @trigger
+      root = parent;
+    } else if (nodeSet.has(parent)) {  // ngIf inside ngIf
+      root = NULL_NODE;
+    } else {  // recurse upwards
+      root = getRoot(parent);
     }
-    return false;
-  };
-  return isRoot;
+
+    localRootMap.set(node, root);
+    return root;
+  }
+
+  nodes.forEach(node => {
+    const root = getRoot(node);
+    if (root !== NULL_NODE) {
+      rootMap.get(root) !.push(node);
+    }
+  });
+
+  return rootMap;
 }
 
 const CLASSES_CACHE_KEY = '$$classes';

packages/animations/browser/test/dsl/animation_trigger_spec.ts

@@ -10,6 +10,7 @@ import {AnimationOptions, animate, state, style, transition} from '@angular/anim
 import {AnimationTransitionInstruction} from '@angular/animations/browser/src/dsl/animation_transition_instruction';
 import {AnimationTrigger} from '@angular/animations/browser/src/dsl/animation_trigger';
 
+import {ENTER_CLASSNAME} from '../../src/util';
 import {MockAnimationDriver} from '../../testing';
 import {makeTrigger} from '../shared';
 
@@ -228,7 +229,8 @@ function buildTransition(
   const trans = trigger.matchTransition(fromState, toState) !;
   if (trans) {
     const driver = new MockAnimationDriver();
-    return trans.build(driver, element, fromState, toState, fromOptions, toOptions) !;
+    return trans.build(
+        driver, element, fromState, toState, ENTER_CLASSNAME, fromOptions, toOptions) !;
   }
   return null;
 }