From 6b4c24020ddd80a9ef135f82d91f51623d1df0d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Tue, 28 Nov 2017 15:08:31 -0600 Subject: [PATCH] Revert "fix(animations): ensure multi-level leave animations work (#19455)" This reverts commit 1366762d12e0faa857deb793255099bfe0e0f717. --- .../animations/browser/src/dsl/animation.ts | 6 +- .../browser/src/dsl/animation_ast_builder.ts | 6 +- .../src/dsl/animation_timeline_builder.ts | 27 ++++---- .../src/dsl/animation_transition_factory.ts | 7 +- .../src/render/timeline_animation_engine.ts | 6 +- .../src/render/transition_animation_engine.ts | 45 +++---------- .../test/dsl/animation_trigger_spec.ts | 5 +- .../animation_query_integration_spec.ts | 66 ------------------- 8 files changed, 36 insertions(+), 132 deletions(-) diff --git a/packages/animations/browser/src/dsl/animation.ts b/packages/animations/browser/src/dsl/animation.ts index e21494a980..c31ffb4eb8 100644 --- a/packages/animations/browser/src/dsl/animation.ts +++ b/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 {ENTER_CLASSNAME, LEAVE_CLASSNAME, normalizeStyles} from '../util'; +import {ENTER_CLASSNAME, normalizeStyles} from '../util'; import {Ast} from './animation_ast'; import {buildAnimationAst} from './animation_ast_builder'; @@ -39,8 +39,8 @@ export class Animation { const errors: any = []; subInstructions = subInstructions || new ElementInstructionMap(); const result = buildAnimationTimelines( - this._driver, element, this._animationAst, ENTER_CLASSNAME, LEAVE_CLASSNAME, 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); diff --git a/packages/animations/browser/src/dsl/animation_ast_builder.ts b/packages/animations/browser/src/dsl/animation_ast_builder.ts index 4884072fc3..20f490c999 100644 --- a/packages/animations/browser/src/dsl/animation_ast_builder.ts +++ b/packages/animations/browser/src/dsl/animation_ast_builder.ts @@ -60,6 +60,8 @@ 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 { @@ -474,8 +476,8 @@ function normalizeSelector(selector: string): [string, boolean] { selector = selector.replace(SELF_TOKEN_REGEX, ''); } - // the :enter and :leave selectors are filled in at runtime during timeline building - selector = selector.replace(/@\*/g, NG_TRIGGER_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); diff --git a/packages/animations/browser/src/dsl/animation_timeline_builder.ts b/packages/animations/browser/src/dsl/animation_timeline_builder.ts index 534cf5c788..df46177a7e 100644 --- a/packages/animations/browser/src/dsl/animation_timeline_builder.ts +++ b/packages/animations/browser/src/dsl/animation_timeline_builder.ts @@ -17,8 +17,6 @@ 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 @@ -105,24 +103,22 @@ const LEAVE_TOKEN_REGEX = new RegExp(LEAVE_TOKEN, 'g'); * the `AnimationValidatorVisitor` code. */ export function buildAnimationTimelines( - driver: AnimationDriver, rootElement: any, ast: Ast, - enterClassName: string, leaveClassName: string, startingStyles: ɵStyleData = {}, - finalStyles: ɵStyleData = {}, options: AnimationOptions, + driver: AnimationDriver, rootElement: any, ast: Ast, enterClassName: string, + startingStyles: ɵStyleData = {}, finalStyles: ɵStyleData = {}, options: AnimationOptions, subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] { return new AnimationTimelineBuilderVisitor().buildKeyframes( - driver, rootElement, ast, enterClassName, leaveClassName, 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, - enterClassName: string, leaveClassName: string, startingStyles: ɵStyleData, - finalStyles: ɵStyleData, options: AnimationOptions, subInstructions?: ElementInstructionMap, - errors: any[] = []): AnimationTimelineInstruction[] { + driver: AnimationDriver, rootElement: any, ast: Ast, enterClassName: string, + startingStyles: ɵStyleData, finalStyles: ɵStyleData, options: AnimationOptions, + subInstructions?: ElementInstructionMap, errors: any[] = []): AnimationTimelineInstruction[] { subInstructions = subInstructions || new ElementInstructionMap(); const context = new AnimationTimelineContext( - driver, rootElement, subInstructions, enterClassName, leaveClassName, errors, []); + driver, rootElement, subInstructions, enterClassName, errors, []); context.options = options; context.currentTimeline.setStyles([startingStyles], null, context.errors, options); @@ -454,7 +450,7 @@ export class AnimationTimelineContext { constructor( private _driver: AnimationDriver, public element: any, public subInstructions: ElementInstructionMap, private _enterClassName: string, - private _leaveClassName: string, public errors: any[], public timelines: TimelineBuilder[], + public errors: any[], public timelines: TimelineBuilder[], initialTimeline?: TimelineBuilder) { this.currentTimeline = initialTimeline || new TimelineBuilder(this._driver, element, 0); timelines.push(this.currentTimeline); @@ -508,8 +504,8 @@ export class AnimationTimelineContext { AnimationTimelineContext { const target = element || this.element; const context = new AnimationTimelineContext( - this._driver, target, this.subInstructions, this._enterClassName, this._leaveClassName, - 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; @@ -565,7 +561,6 @@ 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) { diff --git a/packages/animations/browser/src/dsl/animation_transition_factory.ts b/packages/animations/browser/src/dsl/animation_transition_factory.ts index cd45922eb8..7937f02730 100644 --- a/packages/animations/browser/src/dsl/animation_transition_factory.ts +++ b/packages/animations/browser/src/dsl/animation_transition_factory.ts @@ -37,8 +37,7 @@ export class AnimationTransitionFactory { build( driver: AnimationDriver, element: any, currentState: any, nextState: any, - enterClassName: string, leaveClassName: string, currentOptions?: AnimationOptions, - nextOptions?: AnimationOptions, + enterClassName: string, currentOptions?: AnimationOptions, nextOptions?: AnimationOptions, subInstructions?: ElementInstructionMap): AnimationTransitionInstruction { const errors: any[] = []; @@ -56,8 +55,8 @@ export class AnimationTransitionFactory { const animationOptions = {params: {...transitionAnimationParams, ...nextAnimationParams}}; const timelines = buildAnimationTimelines( - driver, element, this.ast.animation, enterClassName, leaveClassName, currentStateStyles, - nextStateStyles, animationOptions, subInstructions, errors); + driver, element, this.ast.animation, enterClassName, currentStateStyles, nextStateStyles, + animationOptions, subInstructions, errors); if (errors.length) { return createTransitionInstruction( diff --git a/packages/animations/browser/src/render/timeline_animation_engine.ts b/packages/animations/browser/src/render/timeline_animation_engine.ts index d90fc1935a..cbb6c9a55a 100644 --- a/packages/animations/browser/src/render/timeline_animation_engine.ts +++ b/packages/animations/browser/src/render/timeline_animation_engine.ts @@ -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, LEAVE_CLASSNAME} from '../util'; +import {ENTER_CLASSNAME} from '../util'; import {AnimationDriver} from './animation_driver'; import {getOrSetAsInMap, listenOnPlayer, makeAnimationEvent, normalizeKeyframes, optimizeGroupPlayer} from './shared'; @@ -56,8 +56,8 @@ export class TimelineAnimationEngine { if (ast) { instructions = buildAnimationTimelines( - this._driver, element, ast, ENTER_CLASSNAME, LEAVE_CLASSNAME, {}, {}, 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); diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index 565504f025..0e4e6c5001 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -22,8 +22,6 @@ 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 = { @@ -717,11 +715,10 @@ export class TransitionAnimationEngine { } private _buildInstruction( - entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string, - leaveClassName: string) { + entry: QueueInstruction, subTimelines: ElementInstructionMap, enterClassName: string) { return entry.transition.build( this.driver, entry.element, entry.fromState.value, entry.toState.value, enterClassName, - leaveClassName, entry.fromState.options, entry.toState.options, subTimelines); + entry.fromState.options, entry.toState.options, subTimelines); } destroyInnerAnimations(containerElement: any) { @@ -802,13 +799,6 @@ 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[] = []; @@ -873,8 +863,8 @@ export class TransitionAnimationEngine { }); const bodyNode = getBodyNode(); - const allTriggerElements = Array.from(this.statesByElement.keys()); - const enterNodeMap = buildRootMap(allTriggerElements, 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 @@ -888,42 +878,29 @@ export class TransitionAnimationEngine { }); const allLeaveNodes: any[] = []; - const mergedLeaveNodes = new Set(); const leaveNodesWithoutAnimations = new Set(); 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); - mergedLeaveNodes.add(element); - if (details.hasAnimation) { - this.driver.query(element, STAR_SELECTOR, true).forEach(elm => mergedLeaveNodes.add(elm)); - } else { + if (!details.hasAnimation) { leaveNodesWithoutAnimations.add(element); } } } - const leaveNodeMapIds = new Map(); - 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)); }); - leaveNodeMap.forEach((nodes, root) => { - const className = leaveNodeMapIds.get(root) !; - nodes.forEach(node => removeClass(node, className)); + allLeaveNodes.forEach(element => { + removeClass(element, LEAVE_CLASSNAME); + this.processLeaveNode(element); }); - - allLeaveNodes.forEach(element => { this.processLeaveNode(element); }); }); const allPlayers: TransitionAnimationPlayer[] = []; @@ -940,10 +917,8 @@ export class TransitionAnimationEngine { return; } - const leaveClassName = leaveNodeMapIds.get(element) !; const enterClassName = enterNodeMapIds.get(element) !; - const instruction = - this._buildInstruction(entry, subTimelines, enterClassName, leaveClassName) !; + const instruction = this._buildInstruction(entry, subTimelines, enterClassName) !; if (instruction.errors && instruction.errors.length) { erroneousTransitions.push(instruction); return; diff --git a/packages/animations/browser/test/dsl/animation_trigger_spec.ts b/packages/animations/browser/test/dsl/animation_trigger_spec.ts index 416c231317..9e885e63f3 100644 --- a/packages/animations/browser/test/dsl/animation_trigger_spec.ts +++ b/packages/animations/browser/test/dsl/animation_trigger_spec.ts @@ -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, LEAVE_CLASSNAME} from '../../src/util'; +import {ENTER_CLASSNAME} from '../../src/util'; import {MockAnimationDriver} from '../../testing'; import {makeTrigger} from '../shared'; @@ -230,8 +230,7 @@ function buildTransition( if (trans) { const driver = new MockAnimationDriver(); return trans.build( - driver, element, fromState, toState, ENTER_CLASSNAME, LEAVE_CLASSNAME, fromOptions, - toOptions) !; + driver, element, fromState, toState, ENTER_CLASSNAME, fromOptions, toOptions) !; } return null; } diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index 19c2f022cf..2ca24be857 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -3033,72 +3033,6 @@ 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: ` -
-
-
-
-
-
-
-
-
- ` - }) - 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', () => {