Skip to content

Commit

Permalink
Revert "fix(animations): ensure multi-level enter animations work (an…
Browse files Browse the repository at this point in the history 
…gular#19455)"

This reverts commit dd6237e.
  • Loading branch information
@mhevery @wKoza
mhevery authored and wKoza committed Dec 2, 2017
1 parent 5a35243 commit d0e95e1
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 165 deletions.
5 changes: 2 additions & 3 deletions packages/animations/browser/src/dsl/animation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import {AnimationMetadata, AnimationMetadataType, AnimationOptions, ɵStyleData} from '@angular/animations';

import {AnimationDriver} from '../render/animation_driver';
import {ENTER_CLASSNAME, normalizeStyles} from '../util';
import {normalizeStyles} from '../util';

import {Ast} from './animation_ast';
import {buildAnimationAst} from './animation_ast_builder';
Expand Down Expand Up @@ -39,8 +39,7 @@ export class Animation {
const errors: any = [];
subInstructions = subInstructions || new ElementInstructionMap();
const result = buildAnimationTimelines(
this._driver, element, this._animationAst, ENTER_CLASSNAME, start, dest, options,
subInstructions, errors);
this._driver, element, this._animationAst, start, dest, options, subInstructions, errors);
if (errors.length) {
const errorMessage = `animation building failed:\n${errors.join("\n")}`;
throw new Error(errorMessage);
Expand Down
5 changes: 4 additions & 1 deletion packages/animations/browser/src/dsl/animation_ast_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ 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 {
Expand Down Expand Up @@ -476,7 +478,8 @@ function normalizeSelector(selector: string): [string, boolean] {
selector = selector.replace(SELF_TOKEN_REGEX, '');
}

selector = selector.replace(LEAVE_TOKEN_REGEX, LEAVE_SELECTOR)
selector = selector.replace(ENTER_TOKEN_REGEX, ENTER_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);
Expand Down
22 changes: 8 additions & 14 deletions packages/animations/browser/src/dsl/animation_timeline_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ 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
Expand Down Expand Up @@ -103,22 +101,20 @@ const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
* the `AnimationValidatorVisitor` code.
*/
export function buildAnimationTimelines(
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
startingStyles: ɵStyleData = {}, finalStyles: ɵStyleData = {}, options: AnimationOptions,
subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
return new AnimationTimelineBuilderVisitor().buildKeyframes(
driver, rootElement, ast, enterClassName, startingStyles, finalStyles, options,
subInstructions, errors);
driver, rootElement, ast, startingStyles, finalStyles, options, subInstructions, errors);
}

export class AnimationTimelineBuilderVisitor implements AstVisitor {
buildKeyframes(
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
startingStyles: ɵStyleData, finalStyles: ɵStyleData, options: AnimationOptions,
subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
subInstructions = subInstructions || new ElementInstructionMap();
const context = new AnimationTimelineContext(
driver, rootElement, subInstructions, enterClassName, errors, []);
const context = new AnimationTimelineContext(driver, rootElement, subInstructions, errors, []);
context.options = options;
context.currentTimeline.setStyles([startingStyles], null, context.errors, options);

Expand Down Expand Up @@ -449,9 +445,8 @@ export class AnimationTimelineContext {

constructor(
private _driver: AnimationDriver, public element: any,
public subInstructions: ElementInstructionMap, private _enterClassName: string,
public errors: any[], public timelines: TimelineBuilder[],
initialTimeline?: TimelineBuilder) {
public subInstructions: ElementInstructionMap, public errors: any[],
public timelines: TimelineBuilder[], initialTimeline?: TimelineBuilder) {
this.currentTimeline = initialTimeline || new TimelineBuilder(this._driver, element, 0);
timelines.push(this.currentTimeline);
}
Expand Down Expand Up @@ -504,8 +499,8 @@ export class AnimationTimelineContext {
AnimationTimelineContext {
const target = element || this.element;
const context = new AnimationTimelineContext(
this._driver, target, this.subInstructions, this._enterClassName, this.errors,
this.timelines, this.currentTimeline.fork(target, newTime || 0));
this._driver, target, this.subInstructions, this.errors, this.timelines,
this.currentTimeline.fork(target, newTime || 0));
context.previousNode = this.previousNode;
context.currentAnimateTimings = this.currentAnimateTimings;

Expand Down Expand Up @@ -560,7 +555,6 @@ 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) {
Expand Down
6 changes: 3 additions & 3 deletions packages/animations/browser/src/dsl/animation_transition_factory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class AnimationTransitionFactory {

build(
driver: AnimationDriver, element: any, currentState: any, nextState: any,
enterClassName: string, currentOptions?: AnimationOptions, nextOptions?: AnimationOptions,
currentOptions?: AnimationOptions, nextOptions?: AnimationOptions,
subInstructions?: ElementInstructionMap): AnimationTransitionInstruction {
const errors: any[] = [];

Expand All @@ -55,8 +55,8 @@ export class AnimationTransitionFactory {
const animationOptions = {params: {...transitionAnimationParams, ...nextAnimationParams}};

const timelines = buildAnimationTimelines(
driver, element, this.ast.animation, enterClassName, currentStateStyles, nextStateStyles,
animationOptions, subInstructions, errors);
driver, element, this.ast.animation, currentStateStyles, nextStateStyles, animationOptions,
subInstructions, errors);

if (errors.length) {
return createTransitionInstruction(
Expand Down
4 changes: 1 addition & 3 deletions packages/animations/browser/src/render/timeline_animation_engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ 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';
Expand Down Expand Up @@ -56,8 +55,7 @@ export class TimelineAnimationEngine {

if (ast) {
instructions = buildAnimationTimelines(
this._driver, element, ast, ENTER_CLASSNAME, {}, {}, options, EMPTY_INSTRUCTION_MAP,
errors);
this._driver, element, ast, {}, {}, options, EMPTY_INSTRUCTION_MAP, errors);
instructions.forEach(inst => {
const styles = getOrSetAsInMap(autoStylesMap, inst.element, {});
inst.postStyleProps.forEach(prop => styles[prop] = null);
Expand Down
128 changes: 56 additions & 72 deletions packages/animations/browser/src/render/transition_animation_engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, iteratorToArray, 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 {getBodyNode, getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
Expand Down Expand Up @@ -714,10 +714,9 @@ export class TransitionAnimationEngine {
return () => {};
}

private _buildInstruction(
entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string) {
private _buildInstruction(entry: QueueInstruction, subTimelines: ElementInstructionMap) {
return entry.transition.build(
this.driver, entry.element, entry.fromState.value, entry.toState.value, enterClassName,
this.driver, entry.element, entry.fromState.value, entry.toState.value,
entry.fromState.options, entry.toState.options, subTimelines);
}

Expand Down Expand Up @@ -863,19 +862,16 @@ export class TransitionAnimationEngine {
});

const bodyNode = getBodyNode();
const enterNodeMap =
buildRootMap(Array.from(this.statesByElement.keys()), this.collectedEnterElements);
const allEnterNodes: any[] = this.collectedEnterElements.length ?
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).
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));
});
for (let i = 0; i < allEnterNodes.length; i++) {
addClass(allEnterNodes[i], ENTER_CLASSNAME);
}

const allLeaveNodes: any[] = [];
const leaveNodesWithoutAnimations = new Set<any>();
Expand All @@ -892,11 +888,7 @@ export class TransitionAnimationEngine {
}

cleanupFns.push(() => {
enterNodeMap.forEach((nodes, root) => {
const className = enterNodeMapIds.get(root) !;
nodes.forEach(node => removeClass(node, className));
});

allEnterNodes.forEach(element => removeClass(element, ENTER_CLASSNAME));
allLeaveNodes.forEach(element => {
removeClass(element, LEAVE_CLASSNAME);
this.processLeaveNode(element);
Expand All @@ -917,8 +909,7 @@ export class TransitionAnimationEngine {
return;
}

const enterClassName = enterNodeMapIds.get(element) !;
const instruction = this._buildInstruction(entry, subTimelines, enterClassName) !;
const instruction = this._buildInstruction(entry, subTimelines) !;
if (instruction.errors && instruction.errors.length) {
erroneousTransitions.push(instruction);
return;
Expand Down Expand Up @@ -982,6 +973,18 @@ 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
Expand Down Expand Up @@ -1019,9 +1022,8 @@ export class TransitionAnimationEngine {
});

// POST STAGE: fill the * styles
const postStylesMap = new Map<any, ɵStyleData>();
const allLeaveQueriedNodes = cloakAndComputeStyles(
postStylesMap, this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE);
const [postStylesMap, allLeaveQueriedNodes] = cloakAndComputeStyles(
this.driver, leaveNodesWithoutAnimations, allPostStyleElements, AUTO_STYLE);

allLeaveQueriedNodes.forEach(node => {
if (replacePostStylesAsPre(node, allPreStyleElements, allPostStyleElements)) {
Expand All @@ -1030,11 +1032,10 @@ export class TransitionAnimationEngine {
});

// PRE STAGE: fill the ! styles
const preStylesMap = new Map<any, ɵStyleData>();
enterNodeMap.forEach((nodes, root) => {
cloakAndComputeStyles(
preStylesMap, this.driver, new Set(nodes), allPreStyleElements, PRE_STYLE);
});
const [preStylesMap] = allPreStyleElements.size ?
cloakAndComputeStyles(
this.driver, enterNodesWithoutAnimations, allPreStyleElements, PRE_STYLE) :
[new Map<any, ɵStyleData>()];

replaceNodes.forEach(node => {
const post = postStylesMap.get(node);
Expand Down Expand Up @@ -1504,11 +1505,12 @@ function cloakElement(element: any, value?: string) {
}

function cloakAndComputeStyles(
valuesMap: Map<any, ɵStyleData>, driver: AnimationDriver, elements: Set<any>,
elementPropsMap: Map<any, Set<string>>, defaultStyle: string): any[] {
driver: AnimationDriver, elements: Set<any>, elementPropsMap: Map<any, Set<string>>,
defaultStyle: string): [Map<any, ɵStyleData>, 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) => {
Expand All @@ -1530,57 +1532,39 @@ function cloakAndComputeStyles(
// an index value for the closure (but instead just the value)
let i = 0;
elements.forEach(element => cloakElement(element, cloakVals[i++]));

return failedElements;
return [valuesMap, 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 for each animation @trigger.
out which nodes are roots.
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 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;
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.
const NULL_NODE = 1;
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 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);
}

localRootMap.set(node, root);
return root;
}

nodes.forEach(node => {
const root = getRoot(node);
if (root !== NULL_NODE) {
rootMap.get(root) !.push(node);
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 rootMap;
return false;
};
return isRoot;
}

const CLASSES_CACHE_KEY = '$$classes';
Expand Down
4 changes: 1 addition & 3 deletions packages/animations/browser/test/dsl/animation_trigger_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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';

Expand Down Expand Up @@ -229,8 +228,7 @@ function buildTransition(
const trans = trigger.matchTransition(fromState, toState) !;
if (trans) {
const driver = new MockAnimationDriver();
return trans.build(
driver, element, fromState, toState, ENTER_CLASSNAME, fromOptions, toOptions) !;
return trans.build(driver, element, fromState, toState, fromOptions, toOptions) !;
}
return null;
}
Expand Down
Loading

0 comments on commit d0e95e1

Please sign in to comment.