Skip to content

Commit

Permalink
fix(animations): ensure multi-level leave animations work (angular#19455
Browse files Browse the repository at this point in the history
)

PR Close angular#19455
  • Loading branch information
matsko authored and mhevery committed Nov 29, 2017
1 parent b2a586c commit c2b3792
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 36 deletions.
6 changes: 3 additions & 3 deletions packages/animations/browser/src/dsl/animation.ts
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 {ENTER_CLASSNAME, LEAVE_CLASSNAME, normalizeStyles} from '../util';

import {Ast} from './animation_ast';
import {buildAnimationAst} from './animation_ast_builder';
Expand Down Expand Up @@ -39,8 +39,8 @@ 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, ENTER_CLASSNAME, LEAVE_CLASSNAME, start, dest,
options, subInstructions, errors);
if (errors.length) {
const errorMessage = `animation building failed:\n${errors.join("\n")}`;
throw new Error(errorMessage);
Expand Down
6 changes: 2 additions & 4 deletions packages/animations/browser/src/dsl/animation_ast_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ export function buildAnimationAst(
return new AnimationAstBuilderVisitor(driver).build(metadata, errors);
}

const LEAVE_TOKEN = ':leave';
const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');
const ROOT_SELECTOR = '';

export class AnimationAstBuilderVisitor implements AnimationDslVisitor {
Expand Down Expand Up @@ -476,8 +474,8 @@ function normalizeSelector(selector: string): [string, boolean] {
selector = selector.replace(SELF_TOKEN_REGEX, '');
}

selector = selector.replace(LEAVE_TOKEN_REGEX, LEAVE_SELECTOR)
.replace(/@\*/g, NG_TRIGGER_SELECTOR)
// the :enter and :leave selectors are filled in at runtime during timeline building
selector = selector.replace(/@\*/g, NG_TRIGGER_SELECTOR)
.replace(/@\w+/g, match => NG_TRIGGER_SELECTOR + '-' + match.substr(1))
.replace(/:animating/g, NG_ANIMATING_SELECTOR);

Expand Down
27 changes: 16 additions & 11 deletions packages/animations/browser/src/dsl/animation_timeline_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ 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');
const LEAVE_TOKEN = ':leave';
const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');

/*
* The code within this file aims to generate web-animations-compatible keyframes from Angular's
Expand Down Expand Up @@ -103,22 +105,24 @@ const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
* the `AnimationValidatorVisitor` code.
*/
export function buildAnimationTimelines(
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
startingStyles: ɵStyleData = {}, finalStyles: ɵStyleData = {}, options: AnimationOptions,
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
enterClassName: string, leaveClassName: string, 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, enterClassName, leaveClassName, startingStyles, finalStyles,
options, subInstructions, errors);
}

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

Expand Down Expand Up @@ -450,7 +454,7 @@ export class AnimationTimelineContext {
constructor(
private _driver: AnimationDriver, public element: any,
public subInstructions: ElementInstructionMap, private _enterClassName: string,
public errors: any[], public timelines: TimelineBuilder[],
private _leaveClassName: string, 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 +508,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._enterClassName, this._leaveClassName,
this.errors, this.timelines, this.currentTimeline.fork(target, newTime || 0));
context.previousNode = this.previousNode;
context.currentAnimateTimings = this.currentAnimateTimings;

Expand Down Expand Up @@ -561,6 +565,7 @@ export class AnimationTimelineContext {
}
if (selector.length > 0) { // if :self is only used then the selector is empty
selector = selector.replace(ENTER_TOKEN_REGEX, '.' + this._enterClassName);
selector = selector.replace(LEAVE_TOKEN_REGEX, '.' + this._leaveClassName);
const multi = limit != 1;
let elements = this._driver.query(this.element, selector, multi);
if (limit !== 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export class AnimationTransitionFactory {

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

Expand All @@ -55,8 +56,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, enterClassName, leaveClassName, currentStateStyles,
nextStateStyles, animationOptions, subInstructions, errors);

if (errors.length) {
return createTransitionInstruction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +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 {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '../util';

import {AnimationDriver} from './animation_driver';
import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared';
Expand Down Expand Up @@ -56,8 +56,8 @@ export class TimelineAnimationEngine {

if (ast) {
instructions = buildAnimationTimelines(
this._driver, element, ast, ENTER_CLASSNAME, {}, {}, options, EMPTY_INSTRUCTION_MAP,
errors);
this._driver, element, ast, ENTER_CLASSNAME, LEAVE_CLASSNAME, {}, {}, options,
EMPTY_INSTRUCTION_MAP, errors);
instructions.forEach(inst => {
const styles = getOrSetAsInMap(autoStylesMap, inst.element, {});
inst.postStyleProps.forEach(prop => styles[prop] = null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const QUEUED_CLASSNAME = 'ng-animate-queued';
const QUEUED_SELECTOR = '.ng-animate-queued';
const DISABLED_CLASSNAME = 'ng-animate-disabled';
const DISABLED_SELECTOR = '.ng-animate-disabled';
const STAR_CLASSNAME = 'ng-star-inserted';
const STAR_SELECTOR = '.ng-star-inserted';

const EMPTY_PLAYER_ARRAY: TransitionAnimationPlayer[] = [];
const NULL_REMOVAL_STATE: ElementAnimationState = {
Expand Down Expand Up @@ -715,10 +717,11 @@ export class TransitionAnimationEngine {
}

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

destroyInnerAnimations(containerElement: any) {
Expand Down Expand Up @@ -799,6 +802,13 @@ export class TransitionAnimationEngine {
this.newHostElements.clear();
}

if (this.totalAnimations && this.collectedEnterElements.length) {
for (let i = 0; i < this.collectedEnterElements.length; i++) {
const elm = this.collectedEnterElements[i];
addClass(elm, STAR_CLASSNAME);
}
}

if (this._namespaceList.length &&
(this.totalQueuedPlayers || this.collectedLeaveElements.length)) {
const cleanupFns: Function[] = [];
Expand Down Expand Up @@ -863,8 +873,8 @@ export class TransitionAnimationEngine {
});

const bodyNode = getBodyNode();
const enterNodeMap =
buildRootMap(Array.from(this.statesByElement.keys()), this.collectedEnterElements);
const allTriggerElements = Array.from(this.statesByElement.keys());
const enterNodeMap = buildRootMap(allTriggerElements, this.collectedEnterElements);

// this must occur before the instructions are built below such that
// the :enter queries match the elements (since the timeline queries
Expand All @@ -878,29 +888,42 @@ export class TransitionAnimationEngine {
});

const allLeaveNodes: any[] = [];
const mergedLeaveNodes = new Set<any>();
const leaveNodesWithoutAnimations = new Set<any>();
for (let i = 0; i < this.collectedLeaveElements.length; i++) {
const element = this.collectedLeaveElements[i];
const details = element[REMOVAL_FLAG] as ElementAnimationState;
if (details && details.setForRemoval) {
addClass(element, LEAVE_CLASSNAME);
allLeaveNodes.push(element);
if (!details.hasAnimation) {
mergedLeaveNodes.add(element);
if (details.hasAnimation) {
this.driver.query(element, STAR_SELECTOR, true).forEach(elm => mergedLeaveNodes.add(elm));
} else {
leaveNodesWithoutAnimations.add(element);
}
}
}

const leaveNodeMapIds = new Map<any, string>();
const leaveNodeMap = buildRootMap(allTriggerElements, Array.from(mergedLeaveNodes));
leaveNodeMap.forEach((nodes, root) => {
const className = LEAVE_CLASSNAME + i++;
leaveNodeMapIds.set(root, className);
nodes.forEach(node => addClass(node, className));
});

cleanupFns.push(() => {
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);
leaveNodeMap.forEach((nodes, root) => {
const className = leaveNodeMapIds.get(root) !;
nodes.forEach(node => removeClass(node, className));
});

allLeaveNodes.forEach(element => { this.processLeaveNode(element); });
});

const allPlayers: TransitionAnimationPlayer[] = [];
Expand All @@ -917,8 +940,10 @@ export class TransitionAnimationEngine {
return;
}

const leaveClassName = leaveNodeMapIds.get(element) !;
const enterClassName = enterNodeMapIds.get(element) !;
const instruction = this._buildInstruction(entry, subTimelines, enterClassName) !;
const instruction =
this._buildInstruction(entry, subTimelines, enterClassName, leaveClassName) !;
if (instruction.errors && instruction.errors.length) {
erroneousTransitions.push(instruction);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +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 {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '../../src/util';
import {MockAnimationDriver} from '../../testing';
import {makeTrigger} from '../shared';

Expand Down Expand Up @@ -230,7 +230,8 @@ function buildTransition(
if (trans) {
const driver = new MockAnimationDriver();
return trans.build(
driver, element, fromState, toState, ENTER_CLASSNAME, fromOptions, toOptions) !;
driver, element, fromState, toState, ENTER_CLASSNAME, LEAVE_CLASSNAME, fromOptions,
toOptions) !;
}
return null;
}
Expand Down
66 changes: 66 additions & 0 deletions packages/core/test/animation/animation_query_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3033,6 +3033,72 @@ export function main() {
expect(p1.element.classList.contains('container')).toBeTruthy();
expect(p2.element.classList.contains('item')).toBeTruthy();
});

it('should scope :leave queries between sub animations', () => {
@Component({
selector: 'cmp',
animations: [
trigger(
'parent',
[
transition(':leave', group([
sequence([
style({opacity: 0}),
animate(1000, style({opacity: 1})),
]),
query(':leave @child', animateChild()),
])),
]),
trigger(
'child',
[
transition(
':leave',
[
query(
':leave .item',
[style({opacity: 0}), animate(1000, style({opacity: 1}))]),
]),
]),
],
template: `
<div @parent *ngIf="exp1" class="container">
<div *ngIf="exp2">
<div @child>
<div *ngIf="exp3">
<div class="item"></div>
</div>
</div>
</div>
</div>
`
})
class Cmp {
public exp1: any;
public exp2: any;
public exp3: any;
}

TestBed.configureTestingModule({declarations: [Cmp]});

const fixture = TestBed.createComponent(Cmp);
const cmp = fixture.componentInstance;
cmp.exp1 = true;
cmp.exp2 = true;
cmp.exp3 = true;
fixture.detectChanges();
resetLog();

cmp.exp1 = false;
fixture.detectChanges();

const players = getLog();
expect(players.length).toEqual(2);

const [p1, p2] = players;
expect(p1.element.classList.contains('container')).toBeTruthy();
expect(p2.element.classList.contains('item')).toBeTruthy();
});
});

describe('animation control flags', () => {
Expand Down

0 comments on commit c2b3792

Please sign in to comment.