From dcb44e6dea71c489ef9e5f0b9c636b957345956b Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Thu, 6 Dec 2018 15:57:52 -0800 Subject: [PATCH] fix(ivy): generate proper event listener names for animation events (FW-800) (#27525) Prior to this change, animation event names were treated as a regular event names, stripping `@` symbol and event phase. As a result, event listeners were not invoked during animations. Now animation event name is formatted as needed and the necessary callbacks are invoked. PR Close #27525 --- .../r3_view_compiler_styling_spec.ts | 58 +++++++ packages/compiler/src/render3/r3_ast.ts | 6 +- .../compiler/src/render3/view/template.ts | 11 +- .../test/animation_renderer_spec.ts | 67 ++++---- .../test/noop_animations_module_spec.ts | 148 +++++++++--------- 5 files changed, 171 insertions(+), 119 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index b6e47b29fe..71d418141d 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -239,6 +239,64 @@ describe('compiler compliance: styling', () => { const result = compile(files, angularFiles); expectEmit(result.source, template, 'Incorrect template'); }); + + it('should generate animation listeners', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-cmp', + template: \` +
+ \`, + animations: [trigger( + 'myAnimation', + [transition( + '* => state', + [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], + }) + class MyComponent { + exp: any; + startEvent: any; + doneEvent: any; + onStart(event: any) { this.startEvent = event; } + onDone(event: any) { this.doneEvent = event; } + } + + @NgModule({declarations: [MyComponent]}) + export class MyModule {} + ` + } + }; + + const template = ` + … + MyComponent.ngComponentDef = $r3$.ɵdefineComponent({ + … + consts: 1, + vars: 1, + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵelementStart(0, "div", _c0); + $r3$.ɵlistener("@myAnimation.start", function MyComponent_Template_div__myAnimation_start_listener($event) { return ctx.onStart($event); }); + $r3$.ɵlistener("@myAnimation.done", function MyComponent_Template_div__myAnimation_done_listener($event) { return ctx.onDone($event); }); + $r3$.ɵelementEnd(); + } if (rf & 2) { + $r3$.ɵelementProperty(0, "@myAnimation", $r3$.ɵbind(ctx.exp)); + } + }, + encapsulation: 2, + … + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); }); describe('[style] and [style.prop]', () => { diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index e524e157b1..ec7a355aa4 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -49,14 +49,14 @@ export class BoundAttribute implements Node { export class BoundEvent implements Node { constructor( - public name: string, public handler: AST, public target: string|null, - public phase: string|null, public sourceSpan: ParseSourceSpan) {} + public name: string, public type: ParsedEventType, public handler: AST, + public target: string|null, public phase: string|null, public sourceSpan: ParseSourceSpan) {} static fromParsedEvent(event: ParsedEvent) { const target: string|null = event.type === ParsedEventType.Regular ? event.targetOrPhase : null; const phase: string|null = event.type === ParsedEventType.Animation ? event.targetOrPhase : null; - return new BoundEvent(event.name, event.handler, target, phase, event.sourceSpan); + return new BoundEvent(event.name, event.type, event.handler, target, phase, event.sourceSpan); } visit(visitor: Visitor): Result { return visitor.visitBoundEvent(this); } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 69f2e89dff..fbdbb78e72 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -10,7 +10,7 @@ import {flatten, sanitizeIdentifier} from '../../compile_metadata'; import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter'; import {ConstantPool} from '../../constant_pool'; import * as core from '../../core'; -import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, PropertyRead} from '../../expression_parser/ast'; +import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast'; import {Lexer} from '../../expression_parser/lexer'; import {Parser} from '../../expression_parser/parser'; import * as i18n from '../../i18n/i18n_ast'; @@ -987,7 +987,11 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } private prepareListenerParameter(tagName: string, outputAst: t.BoundEvent): () => o.Expression[] { - const evNameSanitized = sanitizeIdentifier(outputAst.name); + let eventName: string = outputAst.name; + if (outputAst.type === ParsedEventType.Animation) { + eventName = prepareSyntheticAttributeName(`${outputAst.name}.${outputAst.phase}`); + } + const evNameSanitized = sanitizeIdentifier(eventName); const tagNameSanitized = sanitizeIdentifier(tagName); const functionName = `${this.templateName}_${tagNameSanitized}_${evNameSanitized}_listener`; @@ -1007,8 +1011,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const handler = o.fn( [new o.FnParam('$event', o.DYNAMIC_TYPE)], statements, o.INFERRED_TYPE, null, functionName); - - return [o.literal(outputAst.name), handler]; + return [o.literal(eventName), handler]; }; } } diff --git a/packages/platform-browser/animations/test/animation_renderer_spec.ts b/packages/platform-browser/animations/test/animation_renderer_spec.ts index cbd27fee4c..8dafb4e600 100644 --- a/packages/platform-browser/animations/test/animation_renderer_spec.ts +++ b/packages/platform-browser/animations/test/animation_renderer_spec.ts @@ -120,43 +120,42 @@ import {el} from '../../testing/src/browser_util'; // these tests are only mean't to be run within the DOM if (isNode) return; - fixmeIvy(`FW-800: Animation listeners are not invoked`) - .it('should flush and fire callbacks when the zone becomes stable', (async) => { - @Component({ - selector: 'my-cmp', - template: '
', - animations: [trigger( - 'myAnimation', - [transition( - '* => state', - [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], - }) - class Cmp { - exp: any; - event: any; - onStart(event: any) { this.event = event; } - } + it('should flush and fire callbacks when the zone becomes stable', (async) => { + @Component({ + selector: 'my-cmp', + template: '
', + animations: [trigger( + 'myAnimation', + [transition( + '* => state', + [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], + }) + class Cmp { + exp: any; + event: any; + onStart(event: any) { this.event = event; } + } - TestBed.configureTestingModule({ - providers: [{provide: AnimationEngine, useClass: InjectableAnimationEngine}], - declarations: [Cmp] - }); + TestBed.configureTestingModule({ + providers: [{provide: AnimationEngine, useClass: InjectableAnimationEngine}], + declarations: [Cmp] + }); - const engine = TestBed.get(AnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; - cmp.exp = 'state'; - fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(cmp.event.triggerName).toEqual('myAnimation'); - expect(cmp.event.phaseName).toEqual('start'); - cmp.event = null; + const engine = TestBed.get(AnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = 'state'; + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(cmp.event.triggerName).toEqual('myAnimation'); + expect(cmp.event.phaseName).toEqual('start'); + cmp.event = null; - engine.flush(); - expect(cmp.event).toBeFalsy(); - async(); - }); - }); + engine.flush(); + expect(cmp.event).toBeFalsy(); + async(); + }); + }); it('should properly insert/remove nodes through the animation renderer that do not contain animations', (async) => { diff --git a/packages/platform-browser/animations/test/noop_animations_module_spec.ts b/packages/platform-browser/animations/test/noop_animations_module_spec.ts index 447c4e952b..d66dace9cb 100644 --- a/packages/platform-browser/animations/test/noop_animations_module_spec.ts +++ b/packages/platform-browser/animations/test/noop_animations_module_spec.ts @@ -10,94 +10,86 @@ import {ɵAnimationEngine} from '@angular/animations/browser'; import {Component} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; -import {fixmeIvy} from '@angular/private/testing'; { describe('NoopAnimationsModule', () => { beforeEach(() => { TestBed.configureTestingModule({imports: [NoopAnimationsModule]}); }); - it('should be removed once FW-800 is fixed', () => { expect(true).toBeTruthy(); }); + it('should flush and fire callbacks when the zone becomes stable', (async) => { + @Component({ + selector: 'my-cmp', + template: + '
', + animations: [trigger( + 'myAnimation', + [transition( + '* => state', [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], + }) + class Cmp { + exp: any; + startEvent: any; + doneEvent: any; + onStart(event: any) { this.startEvent = event; } + onDone(event: any) { this.doneEvent = event; } + } - // TODO: remove the dummy test above ^ once the bug FW-800 has been fixed - fixmeIvy(`FW-800: Animation listeners are not invoked`) - .it('should flush and fire callbacks when the zone becomes stable', (async) => { - @Component({ - selector: 'my-cmp', - template: - '
', - animations: [trigger( - 'myAnimation', - [transition( - '* => state', - [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], - }) - class Cmp { - exp: any; - startEvent: any; - doneEvent: any; - onStart(event: any) { this.startEvent = event; } - onDone(event: any) { this.doneEvent = event; } - } + TestBed.configureTestingModule({declarations: [Cmp]}); - TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = 'state'; + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(cmp.startEvent.triggerName).toEqual('myAnimation'); + expect(cmp.startEvent.phaseName).toEqual('start'); + expect(cmp.doneEvent.triggerName).toEqual('myAnimation'); + expect(cmp.doneEvent.phaseName).toEqual('done'); + async(); + }); + }); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; - cmp.exp = 'state'; - fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(cmp.startEvent.triggerName).toEqual('myAnimation'); - expect(cmp.startEvent.phaseName).toEqual('start'); - expect(cmp.doneEvent.triggerName).toEqual('myAnimation'); - expect(cmp.doneEvent.phaseName).toEqual('done'); - async(); - }); - }); + it('should handle leave animation callbacks even if the element is destroyed in the process', + (async) => { + @Component({ + selector: 'my-cmp', + template: + '
', + animations: [trigger( + 'myAnimation', + [transition( + ':leave', [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], + }) + class Cmp { + exp: any; + startEvent: any; + doneEvent: any; + onStart(event: any) { this.startEvent = event; } + onDone(event: any) { this.doneEvent = event; } + } - fixmeIvy(`FW-800: Animation listeners are not invoked`) - .it('should handle leave animation callbacks even if the element is destroyed in the process', - (async) => { - @Component({ - selector: 'my-cmp', - template: - '
', - animations: [trigger( - 'myAnimation', - [transition( - ':leave', - [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], - }) - class Cmp { - exp: any; - startEvent: any; - doneEvent: any; - onStart(event: any) { this.startEvent = event; } - onDone(event: any) { this.doneEvent = event; } - } + TestBed.configureTestingModule({declarations: [Cmp]}); + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; - TestBed.configureTestingModule({declarations: [Cmp]}); - const engine = TestBed.get(ɵAnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; + cmp.exp = true; + fixture.detectChanges(); + fixture.whenStable().then(() => { + cmp.startEvent = null; + cmp.doneEvent = null; - cmp.exp = true; - fixture.detectChanges(); - fixture.whenStable().then(() => { - cmp.startEvent = null; - cmp.doneEvent = null; - - cmp.exp = false; - fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(cmp.startEvent.triggerName).toEqual('myAnimation'); - expect(cmp.startEvent.phaseName).toEqual('start'); - expect(cmp.startEvent.toState).toEqual('void'); - expect(cmp.doneEvent.triggerName).toEqual('myAnimation'); - expect(cmp.doneEvent.phaseName).toEqual('done'); - expect(cmp.doneEvent.toState).toEqual('void'); - async(); - }); - }); - }); + cmp.exp = false; + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(cmp.startEvent.triggerName).toEqual('myAnimation'); + expect(cmp.startEvent.phaseName).toEqual('start'); + expect(cmp.startEvent.toState).toEqual('void'); + expect(cmp.doneEvent.triggerName).toEqual('myAnimation'); + expect(cmp.doneEvent.phaseName).toEqual('done'); + expect(cmp.doneEvent.toState).toEqual('void'); + async(); + }); + }); + }); }); }