Skip to content

Commit

Permalink
fix(core): animations no longer silently exits if the element is not …
Browse files Browse the repository at this point in the history
…apart of the DOM (angular#13763)
  • Loading branch information
matsko authored and IgorMinar committed Jan 5, 2017
1 parent 889b48d commit 21030e9
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 39 deletions.
48 changes: 29 additions & 19 deletions modules/@angular/core/test/animation/animation_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {AnimationPlayer, NoOpAnimationPlayer} from '../../src/animation/animatio
import {AnimationStyles} from '../../src/animation/animation_styles';
import {AnimationTransitionEvent} from '../../src/animation/animation_transition_event';
import {AUTO_STYLE, animate, group, keyframes, sequence, state, style, transition, trigger} from '../../src/animation/metadata';
import {Input} from '../../src/core';
import {isPresent} from '../../src/facade/lang';
import {TestBed, fakeAsync, flushMicrotasks} from '../../testing';
import {MockAnimationPlayer} from '../../testing/mock_animation_player';
Expand Down Expand Up @@ -2243,38 +2244,52 @@ function declareTests({useJit}: {useJit: boolean}) {
});

describe('error handling', () => {
it('should recover if an animation driver or player throws an error during an animation',
if (!getDOM().supportsWebAnimation()) return;

it('should not throw an error when an animation exists within projected content that is not bound to the DOM',
fakeAsync(() => {
TestBed.configureTestingModule({
declarations: [DummyIfCmp],
providers: [{provide: AnimationDriver, useClass: ErroneousAnimationDriver}],
declarations: [DummyIfCmp, DummyLoadingCmp],
providers: [{provide: AnimationDriver, useClass: WebAnimationsDriver}],
imports: [CommonModule]
});
TestBed.overrideComponent(DummyIfCmp, {
set: {
template: `
<div [@myAnimation]="exp" (@myAnimation.start)="callback1($event)" (@myAnimation.done)="callback2($event)"></div>
<dummy-loading-cmp [exp2]="exp">
<div [@myAnimation]="exp ? 'true' : 'false'" (@myAnimation.done)="callback()">world</div>
</dummy-loading-cmp>
`,
animations: [trigger('myAnimation', [transition(
'* => *',
[
animate(1000, style({transform: 'noooooo'})),
style({opacity: 0}),
animate(1000, style({opacity: 1})),
])])]
}
});
TestBed.overrideComponent(
DummyLoadingCmp, {set: {template: `hello <ng-content *ngIf="exp2"></ng-content>`}});

const fixture = TestBed.createComponent(DummyIfCmp);
const cmp = fixture.componentInstance;
let started = false;
let done = false;
cmp.callback1 = (event: AnimationTransitionEvent) => started = true;
cmp.callback2 = (event: AnimationTransitionEvent) => done = true;
const container = fixture.nativeElement;
let animationCalls = 0;
cmp.callback = () => animationCalls++;

cmp.exp = false;
fixture.detectChanges();
flushMicrotasks();

expect(animationCalls).toBe(1);
expect(getDOM().getText(container).trim()).toEqual('hello');

cmp.exp = true;
fixture.detectChanges();
flushMicrotasks();

expect(started).toBe(true);
expect(done).toBe(true);
expect(animationCalls).toBe(2);
expect(getDOM().getText(container).trim()).toMatch(/hello[\s\r\n]+world/m);
}));
});

Expand Down Expand Up @@ -2460,6 +2475,9 @@ class DummyIfCmp {
class DummyLoadingCmp {
exp: any = false;
callback = () => {};

@Input('exp2')
exp2: any = false;
}

@Component({
Expand Down Expand Up @@ -2571,11 +2589,3 @@ class ExtendedWebAnimationsDriver extends WebAnimationsDriver {
return player;
}
}

class ErroneousAnimationDriver extends MockAnimationDriver {
animate(
element: any, startingStyles: AnimationStyles, keyframes: AnimationKeyframe[],
duration: number, delay: number, easing: string): WebAnimationsPlayer {
throw new Error();
}
}
5 changes: 2 additions & 3 deletions modules/@angular/platform-browser/src/dom/dom_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,11 @@ export class DomRenderer implements Renderer {
element: any, startingStyles: AnimationStyles, keyframes: AnimationKeyframe[],
duration: number, delay: number, easing: string,
previousPlayers: AnimationPlayer[] = []): AnimationPlayer {
try {
if (this._rootRenderer.document.body.contains(element)) {
return this._animationDriver.animate(
element, startingStyles, keyframes, duration, delay, easing, previousPlayers);
} catch (e) {
return new NoOpAnimationPlayer();
}
return new NoOpAnimationPlayer();
}
}

Expand Down
2 changes: 0 additions & 2 deletions modules/@angular/platform-server/src/private_import_core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,3 @@ export type RenderDebugInfo = typeof r._RenderDebugInfo;
export const RenderDebugInfo: typeof r.RenderDebugInfo = r.RenderDebugInfo;
export type DebugDomRootRenderer = typeof r._DebugDomRootRenderer;
export const DebugDomRootRenderer: typeof r.DebugDomRootRenderer = r.DebugDomRootRenderer;
export type NoOpAnimationPlayer = typeof r._NoOpAnimationPlayer;
export const NoOpAnimationPlayer: typeof r.NoOpAnimationPlayer = r.NoOpAnimationPlayer;
10 changes: 3 additions & 7 deletions modules/@angular/platform-server/src/server_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {APP_ID, Inject, Injectable, NgZone, RenderComponentType, Renderer, RootR
import {AnimationDriver, DOCUMENT} from '@angular/platform-browser';

import {isBlank, isPresent, stringify} from './facade/lang';
import {AnimationKeyframe, AnimationPlayer, AnimationStyles, NoOpAnimationPlayer, RenderDebugInfo} from './private_import_core';
import {AnimationKeyframe, AnimationPlayer, AnimationStyles, RenderDebugInfo} from './private_import_core';
import {NAMESPACE_URIS, SharedStylesHost, flattenStyles, getDOM, isNamespaced, shimContentAttribute, shimHostAttribute, splitNamespace} from './private_import_platform-browser';

const TEMPLATE_COMMENT_TEXT = 'template bindings={}';
Expand Down Expand Up @@ -208,12 +208,8 @@ export class ServerRenderer implements Renderer {
element: any, startingStyles: AnimationStyles, keyframes: AnimationKeyframe[],
duration: number, delay: number, easing: string,
previousPlayers: AnimationPlayer[] = []): AnimationPlayer {
try {
return this._animationDriver.animate(
element, startingStyles, keyframes, duration, delay, easing, previousPlayers);
} catch (e) {
return new NoOpAnimationPlayer();
}
return this._animationDriver.animate(
element, startingStyles, keyframes, duration, delay, easing, previousPlayers);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AUTO_STYLE, AnimationTransitionEvent, Component, Injector, animate, state, style, transition, trigger} from '@angular/core';
import {AUTO_STYLE, AnimationTransitionEvent, Component, Injector, ViewChild, animate, state, style, transition, trigger} from '@angular/core';
import {DebugDomRootRenderer} from '@angular/core/src/debug/debug_renderer';
import {RootRenderer} from '@angular/core/src/render/api';
import {TestBed, fakeAsync, flushMicrotasks} from '@angular/core/testing';
Expand Down Expand Up @@ -84,7 +84,7 @@ export function main() {
workerRenderStore = new RenderStore();

TestBed.configureTestingModule({
declarations: [AnimationCmp, MultiAnimationCmp],
declarations: [AnimationCmp, MultiAnimationCmp, ContainerAnimationCmp],
providers: [
Serializer, {provide: RenderStore, useValue: workerRenderStore}, {
provide: RootRenderer,
Expand Down Expand Up @@ -231,10 +231,9 @@ export function main() {
return (event: AnimationTransitionEvent) => { log[phaseName] = event; };
}

const f1 = TestBed.createComponent(AnimationCmp);
const f2 = TestBed.createComponent(AnimationCmp);
const cmp1 = f1.componentInstance;
const cmp2 = f2.componentInstance;
const fixture = TestBed.createComponent(ContainerAnimationCmp);
const cmp1 = fixture.componentInstance.compOne;
const cmp2 = fixture.componentInstance.compTwo;

const cmp1Log: {[phaseName: string]: AnimationTransitionEvent} = {};
const cmp2Log: {[phaseName: string]: AnimationTransitionEvent} = {};
Expand All @@ -246,8 +245,7 @@ export function main() {

cmp1.state = 'off';
cmp2.state = 'on';
f1.detectChanges();
f2.detectChanges();
fixture.detectChanges();
flushMicrotasks();

uiDriver.log.shift()['player'].finish();
Expand Down Expand Up @@ -316,6 +314,18 @@ export function main() {
});
}

@Component({
selector: 'container-comp',
template: `
<my-comp #one></my-comp>
<my-comp #two></my-comp>
`
})
class ContainerAnimationCmp {
@ViewChild('one') public compOne: AnimationCmp;

@ViewChild('two') public compTwo: AnimationCmp;
}

@Component({
selector: 'my-comp',
Expand Down

0 comments on commit 21030e9

Please sign in to comment.