Skip to content

Commit c2b3792

Browse files
matskomhevery
authored andcommitted
fix(animations): ensure multi-level leave animations work (angular#19455)
PR Close angular#19455
1 parent b2a586c commit c2b3792

File tree

8 files changed

+132
-36
lines changed

8 files changed

+132
-36
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import {AnimationMetadata, AnimationMetadataType, AnimationOptions, ɵStyleData} from '@angular/animations';
99

1010
import {AnimationDriver} from '../render/animation_driver';
11-
import {ENTER_CLASSNAME, normalizeStyles} from '../util';
11+
import {ENTER_CLASSNAME, LEAVE_CLASSNAME, normalizeStyles} from '../util';
1212

1313
import {Ast} from './animation_ast';
1414
import {buildAnimationAst} from './animation_ast_builder';
@@ -39,8 +39,8 @@ export class Animation {
3939
const errors: any = [];
4040
subInstructions = subInstructions || new ElementInstructionMap();
4141
const result = buildAnimationTimelines(
42-
this._driver, element, this._animationAst, ENTER_CLASSNAME, start, dest, options,
43-
subInstructions, errors);
42+
this._driver, element, this._animationAst, ENTER_CLASSNAME, LEAVE_CLASSNAME, start, dest,
43+
options, subInstructions, errors);
4444
if (errors.length) {
4545
const errorMessage = `animation building failed:\n${errors.join("\n")}`;
4646
throw new Error(errorMessage);

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ export function buildAnimationAst(
6060
return new AnimationAstBuilderVisitor(driver).build(metadata, errors);
6161
}
6262

63-
const LEAVE_TOKEN = ':leave';
64-
const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');
6563
const ROOT_SELECTOR = '';
6664

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

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

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {ElementInstructionMap} from './element_instruction_map';
1717
const ONE_FRAME_IN_MILLISECONDS = 1;
1818
const ENTER_TOKEN = ':enter';
1919
const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
20+
const LEAVE_TOKEN = ':leave';
21+
const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g');
2022

2123
/*
2224
* The code within this file aims to generate web-animations-compatible keyframes from Angular's
@@ -103,22 +105,24 @@ const ENTER_TOKEN_REGEX = new RegExp(ENTER_TOKEN, 'g');
103105
* the `AnimationValidatorVisitor` code.
104106
*/
105107
export function buildAnimationTimelines(
106-
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
107-
startingStyles: ɵStyleData = {}, finalStyles: ɵStyleData = {}, options: AnimationOptions,
108+
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
109+
enterClassName: string, leaveClassName: string, startingStyles: ɵStyleData = {},
110+
finalStyles: ɵStyleData = {}, options: AnimationOptions,
108111
subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
109112
return new AnimationTimelineBuilderVisitor().buildKeyframes(
110-
driver, rootElement, ast, enterClassName, startingStyles, finalStyles, options,
111-
subInstructions, errors);
113+
driver, rootElement, ast, enterClassName, leaveClassName, startingStyles, finalStyles,
114+
options, subInstructions, errors);
112115
}
113116

114117
export class AnimationTimelineBuilderVisitor implements AstVisitor {
115118
buildKeyframes(
116-
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>, enterClassName: string,
117-
startingStyles: ɵStyleData, finalStyles: ɵStyleData, options: AnimationOptions,
118-
subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] {
119+
driver: AnimationDriver, rootElement: any, ast: Ast<AnimationMetadataType>,
120+
enterClassName: string, leaveClassName: string, startingStyles: ɵStyleData,
121+
finalStyles: ɵStyleData, options: AnimationOptions, subInstructions?: ElementInstructionMap,
122+
errors: any[] = []): AnimationTimelineInstruction[] {
119123
subInstructions = subInstructions || new ElementInstructionMap();
120124
const context = new AnimationTimelineContext(
121-
driver, rootElement, subInstructions, enterClassName, errors, []);
125+
driver, rootElement, subInstructions, enterClassName, leaveClassName, errors, []);
122126
context.options = options;
123127
context.currentTimeline.setStyles([startingStyles], null, context.errors, options);
124128

@@ -450,7 +454,7 @@ export class AnimationTimelineContext {
450454
constructor(
451455
private _driver: AnimationDriver, public element: any,
452456
public subInstructions: ElementInstructionMap, private _enterClassName: string,
453-
public errors: any[], public timelines: TimelineBuilder[],
457+
private _leaveClassName: string, public errors: any[], public timelines: TimelineBuilder[],
454458
initialTimeline?: TimelineBuilder) {
455459
this.currentTimeline = initialTimeline || new TimelineBuilder(this._driver, element, 0);
456460
timelines.push(this.currentTimeline);
@@ -504,8 +508,8 @@ export class AnimationTimelineContext {
504508
AnimationTimelineContext {
505509
const target = element || this.element;
506510
const context = new AnimationTimelineContext(
507-
this._driver, target, this.subInstructions, this._enterClassName, this.errors,
508-
this.timelines, this.currentTimeline.fork(target, newTime || 0));
511+
this._driver, target, this.subInstructions, this._enterClassName, this._leaveClassName,
512+
this.errors, this.timelines, this.currentTimeline.fork(target, newTime || 0));
509513
context.previousNode = this.previousNode;
510514
context.currentAnimateTimings = this.currentAnimateTimings;
511515

@@ -561,6 +565,7 @@ export class AnimationTimelineContext {
561565
}
562566
if (selector.length > 0) { // if :self is only used then the selector is empty
563567
selector = selector.replace(ENTER_TOKEN_REGEX, '.' + this._enterClassName);
568+
selector = selector.replace(LEAVE_TOKEN_REGEX, '.' + this._leaveClassName);
564569
const multi = limit != 1;
565570
let elements = this._driver.query(this.element, selector, multi);
566571
if (limit !== 0) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ export class AnimationTransitionFactory {
3737

3838
build(
3939
driver: AnimationDriver, element: any, currentState: any, nextState: any,
40-
enterClassName: string, currentOptions?: AnimationOptions, nextOptions?: AnimationOptions,
40+
enterClassName: string, leaveClassName: string, currentOptions?: AnimationOptions,
41+
nextOptions?: AnimationOptions,
4142
subInstructions?: ElementInstructionMap): AnimationTransitionInstruction {
4243
const errors: any[] = [];
4344

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

5758
const timelines = buildAnimationTimelines(
58-
driver, element, this.ast.animation, enterClassName, currentStateStyles, nextStateStyles,
59-
animationOptions, subInstructions, errors);
59+
driver, element, this.ast.animation, enterClassName, leaveClassName, currentStateStyles,
60+
nextStateStyles, animationOptions, subInstructions, errors);
6061

6162
if (errors.length) {
6263
return createTransitionInstruction(

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {buildAnimationTimelines} from '../dsl/animation_timeline_builder';
1313
import {AnimationTimelineInstruction} from '../dsl/animation_timeline_instruction';
1414
import {ElementInstructionMap} from '../dsl/element_instruction_map';
1515
import {AnimationStyleNormalizer} from '../dsl/style_normalization/animation_style_normalizer';
16-
import {ENTER_CLASSNAME} from '../util';
16+
import {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '../util';
1717

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

5757
if (ast) {
5858
instructions = buildAnimationTimelines(
59-
this._driver, element, ast, ENTER_CLASSNAME, {}, {}, options, EMPTY_INSTRUCTION_MAP,
60-
errors);
59+
this._driver, element, ast, ENTER_CLASSNAME, LEAVE_CLASSNAME, {}, {}, options,
60+
EMPTY_INSTRUCTION_MAP, errors);
6161
instructions.forEach(inst => {
6262
const styles = getOrSetAsInMap(autoStylesMap, inst.element, {});
6363
inst.postStyleProps.forEach(prop => styles[prop] = null);

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

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ const QUEUED_CLASSNAME = 'ng-animate-queued';
2222
const QUEUED_SELECTOR = '.ng-animate-queued';
2323
const DISABLED_CLASSNAME = 'ng-animate-disabled';
2424
const DISABLED_SELECTOR = '.ng-animate-disabled';
25+
const STAR_CLASSNAME = 'ng-star-inserted';
26+
const STAR_SELECTOR = '.ng-star-inserted';
2527

2628
const EMPTY_PLAYER_ARRAY: TransitionAnimationPlayer[] = [];
2729
const NULL_REMOVAL_STATE: ElementAnimationState = {
@@ -715,10 +717,11 @@ export class TransitionAnimationEngine {
715717
}
716718

717719
private _buildInstruction(
718-
entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string) {
720+
entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string,
721+
leaveClassName: string) {
719722
return entry.transition.build(
720723
this.driver, entry.element, entry.fromState.value, entry.toState.value, enterClassName,
721-
entry.fromState.options, entry.toState.options, subTimelines);
724+
leaveClassName, entry.fromState.options, entry.toState.options, subTimelines);
722725
}
723726

724727
destroyInnerAnimations(containerElement: any) {
@@ -799,6 +802,13 @@ export class TransitionAnimationEngine {
799802
this.newHostElements.clear();
800803
}
801804

805+
if (this.totalAnimations && this.collectedEnterElements.length) {
806+
for (let i = 0; i < this.collectedEnterElements.length; i++) {
807+
const elm = this.collectedEnterElements[i];
808+
addClass(elm, STAR_CLASSNAME);
809+
}
810+
}
811+
802812
if (this._namespaceList.length &&
803813
(this.totalQueuedPlayers || this.collectedLeaveElements.length)) {
804814
const cleanupFns: Function[] = [];
@@ -863,8 +873,8 @@ export class TransitionAnimationEngine {
863873
});
864874

865875
const bodyNode = getBodyNode();
866-
const enterNodeMap =
867-
buildRootMap(Array.from(this.statesByElement.keys()), this.collectedEnterElements);
876+
const allTriggerElements = Array.from(this.statesByElement.keys());
877+
const enterNodeMap = buildRootMap(allTriggerElements, this.collectedEnterElements);
868878

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

880890
const allLeaveNodes: any[] = [];
891+
const mergedLeaveNodes = new Set<any>();
881892
const leaveNodesWithoutAnimations = new Set<any>();
882893
for (let i = 0; i < this.collectedLeaveElements.length; i++) {
883894
const element = this.collectedLeaveElements[i];
884895
const details = element[REMOVAL_FLAG] as ElementAnimationState;
885896
if (details && details.setForRemoval) {
886-
addClass(element, LEAVE_CLASSNAME);
887897
allLeaveNodes.push(element);
888-
if (!details.hasAnimation) {
898+
mergedLeaveNodes.add(element);
899+
if (details.hasAnimation) {
900+
this.driver.query(element, STAR_SELECTOR, true).forEach(elm => mergedLeaveNodes.add(elm));
901+
} else {
889902
leaveNodesWithoutAnimations.add(element);
890903
}
891904
}
892905
}
893906

907+
const leaveNodeMapIds = new Map<any, string>();
908+
const leaveNodeMap = buildRootMap(allTriggerElements, Array.from(mergedLeaveNodes));
909+
leaveNodeMap.forEach((nodes, root) => {
910+
const className = LEAVE_CLASSNAME + i++;
911+
leaveNodeMapIds.set(root, className);
912+
nodes.forEach(node => addClass(node, className));
913+
});
914+
894915
cleanupFns.push(() => {
895916
enterNodeMap.forEach((nodes, root) => {
896917
const className = enterNodeMapIds.get(root) !;
897918
nodes.forEach(node => removeClass(node, className));
898919
});
899920

900-
allLeaveNodes.forEach(element => {
901-
removeClass(element, LEAVE_CLASSNAME);
902-
this.processLeaveNode(element);
921+
leaveNodeMap.forEach((nodes, root) => {
922+
const className = leaveNodeMapIds.get(root) !;
923+
nodes.forEach(node => removeClass(node, className));
903924
});
925+
926+
allLeaveNodes.forEach(element => { this.processLeaveNode(element); });
904927
});
905928

906929
const allPlayers: TransitionAnimationPlayer[] = [];
@@ -917,8 +940,10 @@ export class TransitionAnimationEngine {
917940
return;
918941
}
919942

943+
const leaveClassName = leaveNodeMapIds.get(element) !;
920944
const enterClassName = enterNodeMapIds.get(element) !;
921-
const instruction = this._buildInstruction(entry, subTimelines, enterClassName) !;
945+
const instruction =
946+
this._buildInstruction(entry, subTimelines, enterClassName, leaveClassName) !;
922947
if (instruction.errors && instruction.errors.length) {
923948
erroneousTransitions.push(instruction);
924949
return;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {AnimationOptions, animate, state, style, transition} from '@angular/anim
1010
import {AnimationTransitionInstruction} from '@angular/animations/browser/src/dsl/animation_transition_instruction';
1111
import {AnimationTrigger} from '@angular/animations/browser/src/dsl/animation_trigger';
1212

13-
import {ENTER_CLASSNAME} from '../../src/util';
13+
import {ENTER_CLASSNAME, LEAVE_CLASSNAME} from '../../src/util';
1414
import {MockAnimationDriver} from '../../testing';
1515
import {makeTrigger} from '../shared';
1616

@@ -230,7 +230,8 @@ function buildTransition(
230230
if (trans) {
231231
const driver = new MockAnimationDriver();
232232
return trans.build(
233-
driver, element, fromState, toState, ENTER_CLASSNAME, fromOptions, toOptions) !;
233+
driver, element, fromState, toState, ENTER_CLASSNAME, LEAVE_CLASSNAME, fromOptions,
234+
toOptions) !;
234235
}
235236
return null;
236237
}

packages/core/test/animation/animation_query_integration_spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3033,6 +3033,72 @@ export function main() {
30333033
expect(p1.element.classList.contains('container')).toBeTruthy();
30343034
expect(p2.element.classList.contains('item')).toBeTruthy();
30353035
});
3036+
3037+
it('should scope :leave queries between sub animations', () => {
3038+
@Component({
3039+
selector: 'cmp',
3040+
animations: [
3041+
trigger(
3042+
'parent',
3043+
[
3044+
transition(':leave', group([
3045+
sequence([
3046+
style({opacity: 0}),
3047+
animate(1000, style({opacity: 1})),
3048+
]),
3049+
query(':leave @child', animateChild()),
3050+
])),
3051+
]),
3052+
trigger(
3053+
'child',
3054+
[
3055+
transition(
3056+
':leave',
3057+
[
3058+
query(
3059+
':leave .item',
3060+
[style({opacity: 0}), animate(1000, style({opacity: 1}))]),
3061+
]),
3062+
]),
3063+
],
3064+
template: `
3065+
<div @parent *ngIf="exp1" class="container">
3066+
<div *ngIf="exp2">
3067+
<div @child>
3068+
<div *ngIf="exp3">
3069+
<div class="item"></div>
3070+
</div>
3071+
</div>
3072+
</div>
3073+
</div>
3074+
`
3075+
})
3076+
class Cmp {
3077+
public exp1: any;
3078+
public exp2: any;
3079+
public exp3: any;
3080+
}
3081+
3082+
TestBed.configureTestingModule({declarations: [Cmp]});
3083+
3084+
const fixture = TestBed.createComponent(Cmp);
3085+
const cmp = fixture.componentInstance;
3086+
cmp.exp1 = true;
3087+
cmp.exp2 = true;
3088+
cmp.exp3 = true;
3089+
fixture.detectChanges();
3090+
resetLog();
3091+
3092+
cmp.exp1 = false;
3093+
fixture.detectChanges();
3094+
3095+
const players = getLog();
3096+
expect(players.length).toEqual(2);
3097+
3098+
const [p1, p2] = players;
3099+
expect(p1.element.classList.contains('container')).toBeTruthy();
3100+
expect(p2.element.classList.contains('item')).toBeTruthy();
3101+
});
30363102
});
30373103

30383104
describe('animation control flags', () => {

0 commit comments

Comments
 (0)