fix(ivy): ensure element removal triggers host removal animations (#28162)

Prior to this fix Ivy would not execute any animation triggers
that exist as host bindings on an element if it is removed by
the parent template.

PR Close #28162
This commit is contained in:
Matias Niemelä 2019-01-14 15:36:08 -08:00 committed by Alex Rickabaugh
parent e172e97e13
commit 5a582a8afd
10 changed files with 127 additions and 124 deletions

View File

@ -66,8 +66,8 @@ export class AnimationEngine {
this._transitionEngine.insertNode(namespaceId, element, parent, insertBefore);
}
onRemove(namespaceId: string, element: any, context: any): void {
this._transitionEngine.removeNode(namespaceId, element, context);
onRemove(namespaceId: string, element: any, context: any, isHostElement?: boolean): void {
this._transitionEngine.removeNode(namespaceId, element, isHostElement || false, context);
}
disableAnimations(element: any, disable: boolean) {

View File

@ -708,17 +708,23 @@ export class TransitionAnimationEngine {
}
}
removeNode(namespaceId: string, element: any, context: any): void {
if (!isElementNode(element)) {
this._onRemovalComplete(element, context);
return;
}
removeNode(namespaceId: string, element: any, isHostElement: boolean, context: any): void {
if (isElementNode(element)) {
const ns = namespaceId ? this._fetchNamespace(namespaceId) : null;
if (ns) {
ns.removeNode(element, context);
} else {
this.markElementAsRemoved(namespaceId, element, false, context);
}
const ns = namespaceId ? this._fetchNamespace(namespaceId) : null;
if (ns) {
ns.removeNode(element, context);
if (isHostElement) {
const hostNS = this.namespacesByHostElement.get(element);
if (hostNS && hostNS.id !== namespaceId) {
hostNS.removeNode(element, context);
}
}
} else {
this.markElementAsRemoved(namespaceId, element, false, context);
this._onRemovalComplete(element, context);
}
}

View File

@ -111,7 +111,7 @@ const DEFAULT_NAMESPACE_ID = 'id';
expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();
engine.removeNode(DEFAULT_NAMESPACE_ID, element, true);
engine.removeNode(DEFAULT_NAMESPACE_ID, element, true, true);
engine.flush();
expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();

View File

@ -267,8 +267,10 @@ export abstract class Renderer2 {
* Implement this callback to remove a child node from the host element's DOM.
* @param parent The parent node.
* @param oldChild The child node to remove.
* @param isHostElement Optionally signal to the renderer whether this element is a host element
* or not
*/
abstract removeChild(parent: any, oldChild: any): void;
abstract removeChild(parent: any, oldChild: any, isHostElement?: boolean): void;
/**
* Implement this callback to prepare an element to be bootstrapped
* as a root element, and return the element instance.

View File

@ -74,7 +74,7 @@ export interface ProceduralRenderer3 {
destroyNode?: ((node: RNode) => void)|null;
appendChild(parent: RElement, newChild: RNode): void;
insertBefore(parent: RNode, newChild: RNode, refChild: RNode|null): void;
removeChild(parent: RElement, oldChild: RNode): void;
removeChild(parent: RElement, oldChild: RNode, isHostElement?: boolean): void;
selectRootElement(selectorOrNode: string|any): RElement;
parentNode(node: RNode): RElement|null;

View File

@ -14,7 +14,7 @@ import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'
import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
import {CLEANUP, CONTAINER_INDEX, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
import {assertNodeType} from './node_assert';
import {findComponentView, getNativeByTNode, isLContainer, isRootView, readElementValue, renderStringify} from './util';
import {findComponentView, getNativeByTNode, isComponent, isLContainer, isRootView, readElementValue, renderStringify} from './util';
const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4 + unused5;
@ -84,15 +84,16 @@ function walkTNodeTree(
let nextTNode: TNode|null = null;
if (tNode.type === TNodeType.Element) {
executeNodeAction(
action, renderer, renderParent, getNativeByTNode(tNode, currentView), beforeNode);
action, renderer, renderParent, getNativeByTNode(tNode, currentView), tNode, beforeNode);
const nodeOrContainer = currentView[tNode.index];
if (isLContainer(nodeOrContainer)) {
// This element has an LContainer, and its comment needs to be handled
executeNodeAction(action, renderer, renderParent, nodeOrContainer[NATIVE], beforeNode);
executeNodeAction(
action, renderer, renderParent, nodeOrContainer[NATIVE], tNode, beforeNode);
}
} else if (tNode.type === TNodeType.Container) {
const lContainer = currentView ![tNode.index] as LContainer;
executeNodeAction(action, renderer, renderParent, lContainer[NATIVE], beforeNode);
executeNodeAction(action, renderer, renderParent, lContainer[NATIVE], tNode, beforeNode);
if (lContainer[VIEWS].length) {
currentView = lContainer[VIEWS][0];
@ -166,11 +167,11 @@ function walkTNodeTree(
*/
function executeNodeAction(
action: WalkTNodeTreeAction, renderer: Renderer3, parent: RElement | null,
node: RComment | RElement | RText, beforeNode?: RNode | null) {
node: RComment | RElement | RText, tNode: TNode, beforeNode?: RNode | null) {
if (action === WalkTNodeTreeAction.Insert) {
nativeInsertBefore(renderer, parent !, node, beforeNode || null);
} else if (action === WalkTNodeTreeAction.Detach) {
nativeRemoveChild(renderer, parent !, node);
nativeRemoveChild(renderer, parent !, node, isComponent(tNode));
} else if (action === WalkTNodeTreeAction.Destroy) {
ngDevMode && ngDevMode.rendererDestroyNode++;
(renderer as ProceduralRenderer3).destroyNode !(node);
@ -550,8 +551,9 @@ export function nativeInsertBefore(
/**
* Removes a native child node from a given native parent node.
*/
export function nativeRemoveChild(renderer: Renderer3, parent: RElement, child: RNode): void {
isProceduralRenderer(renderer) ? renderer.removeChild(parent as RElement, child) :
export function nativeRemoveChild(
renderer: Renderer3, parent: RElement, child: RNode, isHostElement?: boolean): void {
isProceduralRenderer(renderer) ? renderer.removeChild(parent as RElement, child, isHostElement) :
parent.removeChild(child);
}

View File

@ -852,58 +852,55 @@ const DEFAULT_COMPONENT_ID = '1';
expect(data.keyframes).toEqual([{offset: 0, opacity: '0'}, {offset: 1, opacity: '1'}]);
}));
// nonAnimationRenderer => animationRenderer
fixmeIvy(
'FW-943 - elements are removed in the wrong renderer so far as host animation @triggers are concerned')
.it('should trigger a leave animation when the inner components host binding updates',
fakeAsync(() => {
@Component({
selector: 'parent-cmp',
template: `
it('should trigger a leave animation when the inner components host binding updates',
fakeAsync(() => {
@Component({
selector: 'parent-cmp',
template: `
<child-cmp *ngIf="exp"></child-cmp>
`
})
class ParentCmp {
public exp = true;
}
})
class ParentCmp {
public exp = true;
}
@Component({
selector: 'child-cmp',
template: '...',
animations: [trigger(
'host', [transition(
':leave',
[style({opacity: 1}), animate(1000, style({opacity: 0}))])])]
})
class ChildCmp {
@HostBinding('@host') public hostAnimation = true;
}
@Component({
selector: 'child-cmp',
template: '...',
animations: [trigger(
'host',
[transition(
':leave', [style({opacity: 1}), animate(1000, style({opacity: 0}))])])]
})
class ChildCmp {
@HostBinding('@host') public hostAnimation = true;
}
TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
const engine = TestBed.get(ɵAnimationEngine);
const fixture = TestBed.createComponent(ParentCmp);
const cmp = fixture.componentInstance;
fixture.detectChanges();
engine.flush();
expect(getLog().length).toEqual(0);
const engine = TestBed.get(ɵAnimationEngine);
const fixture = TestBed.createComponent(ParentCmp);
const cmp = fixture.componentInstance;
fixture.detectChanges();
engine.flush();
expect(getLog().length).toEqual(0);
cmp.exp = false;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.children.length).toBe(1);
cmp.exp = false;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.children.length).toBe(1);
engine.flush();
expect(getLog().length).toEqual(1);
engine.flush();
expect(getLog().length).toEqual(1);
const [player] = getLog();
expect(player.keyframes).toEqual([
{opacity: '1', offset: 0},
{opacity: '0', offset: 1},
]);
const [player] = getLog();
expect(player.keyframes).toEqual([
{opacity: '1', offset: 0},
{opacity: '0', offset: 1},
]);
player.finish();
expect(fixture.debugElement.nativeElement.children.length).toBe(0);
}));
player.finish();
expect(fixture.debugElement.nativeElement.children.length).toBe(0);
}));
// animationRenderer => nonAnimationRenderer
it('should trigger a leave animation when the outer components element binding updates on the host component element',
@ -956,71 +953,67 @@ const DEFAULT_COMPONENT_ID = '1';
expect(fixture.debugElement.nativeElement.children.length).toBe(0);
}));
// animationRenderer => animationRenderer
fixmeIvy(
'FW-943 - elements are removed in the wrong renderer so far as host animation @triggers are concerned')
.it('should trigger a leave animation when both the inner and outer components trigger on the same element',
fakeAsync(() => {
@Component({
selector: 'parent-cmp',
animations: [trigger(
'host',
[transition(
':leave',
[style({height: '100px'}), animate(1000, style({height: '0px'}))])])],
template: `
it('should trigger a leave animation when both the inner and outer components trigger on the same element',
fakeAsync(() => {
@Component({
selector: 'parent-cmp',
animations: [trigger(
'host',
[transition(
':leave',
[style({height: '100px'}), animate(1000, style({height: '0px'}))])])],
template: `
<child-cmp *ngIf="exp" @host></child-cmp>
`
})
class ParentCmp {
public exp = true;
}
})
class ParentCmp {
public exp = true;
}
@Component({
selector: 'child-cmp',
template: '...',
animations: [trigger(
'host',
[transition(
':leave',
[style({width: '100px'}), animate(1000, style({width: '0px'}))])])]
})
class ChildCmp {
@HostBinding('@host') public hostAnimation = true;
}
@Component({
selector: 'child-cmp',
template: '...',
animations: [trigger(
'host', [transition(
':leave',
[style({width: '100px'}), animate(1000, style({width: '0px'}))])])]
})
class ChildCmp {
@HostBinding('@host') public hostAnimation = true;
}
TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
const engine = TestBed.get(ɵAnimationEngine);
const fixture = TestBed.createComponent(ParentCmp);
const cmp = fixture.componentInstance;
fixture.detectChanges();
engine.flush();
expect(getLog().length).toEqual(0);
const engine = TestBed.get(ɵAnimationEngine);
const fixture = TestBed.createComponent(ParentCmp);
const cmp = fixture.componentInstance;
fixture.detectChanges();
engine.flush();
expect(getLog().length).toEqual(0);
cmp.exp = false;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.children.length).toBe(1);
cmp.exp = false;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.children.length).toBe(1);
engine.flush();
expect(getLog().length).toEqual(2);
engine.flush();
expect(getLog().length).toEqual(2);
const [p1, p2] = getLog();
expect(p1.keyframes).toEqual([
{width: '100px', offset: 0},
{width: '0px', offset: 1},
]);
const [p1, p2] = getLog();
expect(p1.keyframes).toEqual([
{width: '100px', offset: 0},
{width: '0px', offset: 1},
]);
expect(p2.keyframes).toEqual([
{height: '100px', offset: 0},
{height: '0px', offset: 1},
]);
expect(p2.keyframes).toEqual([
{height: '100px', offset: 0},
{height: '0px', offset: 1},
]);
p1.finish();
p2.finish();
flushMicrotasks();
expect(fixture.debugElement.nativeElement.children.length).toBe(0);
}));
p1.finish();
p2.finish();
flushMicrotasks();
expect(fixture.debugElement.nativeElement.children.length).toBe(0);
}));
it('should not throw when the host element is removed and no animation triggers',
fakeAsync(() => {

View File

@ -2239,7 +2239,7 @@ import {HostListener} from '../../src/metadata/directives';
});
fixmeIvy(
'FW-943 - elements are removed in the wrong renderer so far as host animation @triggers are concerned')
'FW-943 - Fix final `unknown` issue in `animation_query_integration_spec.ts` once #28162 lands')
.it('should emulate a leave animation on the nearest sub host elements when a parent is removed',
fakeAsync(() => {
@Component({

View File

@ -148,8 +148,8 @@ export class BaseAnimationRenderer implements Renderer2 {
this.engine.onInsert(this.namespaceId, newChild, parent, true);
}
removeChild(parent: any, oldChild: any): void {
this.engine.onRemove(this.namespaceId, oldChild, this.delegate);
removeChild(parent: any, oldChild: any, isHostElement: boolean): void {
this.engine.onRemove(this.namespaceId, oldChild, this.delegate, isHostElement);
}
selectRootElement(selectorOrNode: any, preserveContent?: boolean) {

View File

@ -770,7 +770,7 @@ export declare abstract class Renderer2 {
abstract nextSibling(node: any): any;
abstract parentNode(node: any): any;
abstract removeAttribute(el: any, name: string, namespace?: string | null): void;
abstract removeChild(parent: any, oldChild: any): void;
abstract removeChild(parent: any, oldChild: any, isHostElement?: boolean): void;
abstract removeClass(el: any, name: string): void;
abstract removeStyle(el: any, style: string, flags?: RendererStyleFlags2): void;
abstract selectRootElement(selectorOrNode: string | any, preserveContent?: boolean): any;