Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animations): silently exit an animation if the element is not apart of the DOM #13763

Merged
merged 1 commit into from
Jan 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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