From 1ca22baddf44b354ccbed27f54fc3d89c8bd99b1 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 16 Mar 2019 16:27:01 +0100 Subject: [PATCH] refactor(tree): rework to account for ivy changes Reworks the `MatTree` and `CdkTree` components to account for some of the changes in Ivy. This includes: * Changing the padding directive to account for a different time at which static inputs are initialized. * Changing the toggle directive to account for inheritance of host listeners working differently. * Switching to `Default` change detection, because the way embedded views are checked is slightly different. * Using `descendants: true` to find the nested node outlet in order to handle a slight difference in how indirect content children are matched. --- src/cdk/tree/nested-node.ts | 37 ++++++++++++++++++++++------ src/cdk/tree/outlet.ts | 14 ++++++++++- src/cdk/tree/padding.ts | 25 +++++++++++++------ src/cdk/tree/toggle.ts | 19 +++++++------- src/cdk/tree/tree.ts | 21 ++++++++++++---- src/lib/tree/node.ts | 28 +++++++++++++++------ src/lib/tree/outlet.ts | 8 ++++-- src/lib/tree/toggle.ts | 3 --- src/lib/tree/tree.ts | 4 ++- tools/public_api_guard/cdk/tree.d.ts | 8 ++++-- tools/public_api_guard/lib/tree.d.ts | 3 ++- 11 files changed, 123 insertions(+), 47 deletions(-) diff --git a/src/cdk/tree/nested-node.ts b/src/cdk/tree/nested-node.ts index b8d557c62087..4a8b0ef7f4c3 100644 --- a/src/cdk/tree/nested-node.ts +++ b/src/cdk/tree/nested-node.ts @@ -10,16 +10,16 @@ import { ContentChildren, Directive, ElementRef, - IterableDiffers, IterableDiffer, + IterableDiffers, OnDestroy, QueryList, } from '@angular/core'; import {Observable} from 'rxjs'; import {takeUntil} from 'rxjs/operators'; +import {CDK_TREE_NODE_OUTLET_NODE, CdkTreeNodeOutlet} from './outlet'; import {CdkTree, CdkTreeNode} from './tree'; -import {CdkTreeNodeOutlet} from './outlet'; import {getTreeControlFunctionsMissingError} from './tree-errors'; /** @@ -51,7 +51,10 @@ import {getTreeControlFunctionsMissingError} from './tree-errors'; '[attr.role]': 'role', 'class': 'cdk-tree-node cdk-nested-tree-node', }, - providers: [{provide: CdkTreeNode, useExisting: CdkNestedTreeNode}] + providers: [ + {provide: CdkTreeNode, useExisting: CdkNestedTreeNode}, + {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: CdkNestedTreeNode} + ] }) export class CdkNestedTreeNode extends CdkTreeNode implements AfterContentInit, OnDestroy { /** Differ used to find the changes in the data provided by the data source. */ @@ -61,7 +64,12 @@ export class CdkNestedTreeNode extends CdkTreeNode implements AfterContent protected _children: T[]; /** The children node placeholder. */ - @ContentChildren(CdkTreeNodeOutlet) nodeOutlet: QueryList; + @ContentChildren(CdkTreeNodeOutlet, { + // We need to use `descendants: true`, because Ivy will no longer match + // indirect descendants if it's left as false. + descendants: true + }) + nodeOutlet: QueryList; constructor(protected _elementRef: ElementRef, protected _tree: CdkTree, @@ -92,11 +100,12 @@ export class CdkNestedTreeNode extends CdkTreeNode implements AfterContent /** Add children dataNodes to the NodeOutlet */ protected updateChildrenNodes(children?: T[]): void { + const outlet = this._getNodeOutlet(); if (children) { this._children = children; } - if (this.nodeOutlet.length && this._children) { - const viewContainer = this.nodeOutlet.first.viewContainer; + if (outlet && this._children) { + const viewContainer = outlet.viewContainer; this._tree.renderNodeChanges(this._children, this._dataDiffer, viewContainer, this._data); } else { // Reset the data differ if there's no children nodes displayed @@ -106,9 +115,21 @@ export class CdkNestedTreeNode extends CdkTreeNode implements AfterContent /** Clear the children dataNodes. */ protected _clear(): void { - if (this.nodeOutlet && this.nodeOutlet.first) { - this.nodeOutlet.first.viewContainer.clear(); + const outlet = this._getNodeOutlet(); + if (outlet) { + outlet.viewContainer.clear(); this._dataDiffer.diff([]); } } + + /** Gets the outlet for the current node. */ + private _getNodeOutlet() { + const outlets = this.nodeOutlet; + + if (outlets) { + // Note that since we use `descendants: true` on the query, we have to ensure + // that we don't pick up the outlet of a child node by accident. + return outlets.find(outlet => !outlet._node || outlet._node === this); + } + } } diff --git a/src/cdk/tree/outlet.ts b/src/cdk/tree/outlet.ts index 2895003d893a..26335602815c 100644 --- a/src/cdk/tree/outlet.ts +++ b/src/cdk/tree/outlet.ts @@ -7,9 +7,19 @@ */ import { Directive, + Inject, + InjectionToken, + Optional, ViewContainerRef, } from '@angular/core'; +/** + * Injection token used to provide a `CdkTreeNode` to its outlet. + * Used primarily to avoid circular imports. + * @docs-private + */ +export const CDK_TREE_NODE_OUTLET_NODE = new InjectionToken<{}>('CDK_TREE_NODE_OUTLET_NODE'); + /** * Outlet for nested CdkNode. Put `[cdkTreeNodeOutlet]` on a tag to place children dataNodes * inside the outlet. @@ -18,5 +28,7 @@ import { selector: '[cdkTreeNodeOutlet]' }) export class CdkTreeNodeOutlet { - constructor(public viewContainer: ViewContainerRef) {} + constructor( + public viewContainer: ViewContainerRef, + @Inject(CDK_TREE_NODE_OUTLET_NODE) @Optional() public _node?: any) {} } diff --git a/src/cdk/tree/padding.ts b/src/cdk/tree/padding.ts index 5c8cad8aaa1a..e26b088f9cad 100644 --- a/src/cdk/tree/padding.ts +++ b/src/cdk/tree/padding.ts @@ -24,6 +24,9 @@ const cssUnitPattern = /([A-Za-z%]+)$/; selector: '[cdkTreeNodePadding]', }) export class CdkTreeNodePadding implements OnDestroy { + /** Current padding value applied to the element. Used to avoid unnecessarily hitting the DOM. */ + private _currentPadding: string|null; + /** Subject that emits when the component has been destroyed. */ private _destroyed = new Subject(); @@ -68,8 +71,13 @@ export class CdkTreeNodePadding implements OnDestroy { @Optional() private _dir: Directionality) { this._setPadding(); if (_dir) { - _dir.change.pipe(takeUntil(this._destroyed)).subscribe(() => this._setPadding()); + _dir.change.pipe(takeUntil(this._destroyed)).subscribe(() => this._setPadding(true)); } + + // In Ivy the indentation binding might be set before the tree node's data has been added, + // which means that we'll miss the first render. We have to subscribe to changes in the + // data to ensure that everything is up to date. + _treeNode._dataChanges.subscribe(() => this._setPadding()); } ngOnDestroy() { @@ -86,13 +94,16 @@ export class CdkTreeNodePadding implements OnDestroy { return level ? `${level * this._indent}${this.indentUnits}` : null; } - _setPadding() { - const element = this._element.nativeElement; + _setPadding(forceChange = false) { const padding = this._paddingIndent(); - const paddingProp = this._dir && this._dir.value === 'rtl' ? 'paddingRight' : 'paddingLeft'; - const resetProp = paddingProp === 'paddingLeft' ? 'paddingRight' : 'paddingLeft'; - this._renderer.setStyle(element, paddingProp, padding); - this._renderer.setStyle(element, resetProp, ''); + if (padding !== this._currentPadding || forceChange) { + const element = this._element.nativeElement; + const paddingProp = this._dir && this._dir.value === 'rtl' ? 'paddingRight' : 'paddingLeft'; + const resetProp = paddingProp === 'paddingLeft' ? 'paddingRight' : 'paddingLeft'; + this._renderer.setStyle(element, paddingProp, padding); + this._renderer.setStyle(element, resetProp, null); + this._currentPadding = padding; + } } } diff --git a/src/cdk/tree/toggle.ts b/src/cdk/tree/toggle.ts index ee2956b9d7f9..a8bd4fdafaa4 100644 --- a/src/cdk/tree/toggle.ts +++ b/src/cdk/tree/toggle.ts @@ -7,21 +7,14 @@ */ import {coerceBooleanProperty} from '@angular/cdk/coercion'; -import { - Directive, - Input, -} from '@angular/core'; +import {Directive, HostListener, Input} from '@angular/core'; + import {CdkTree, CdkTreeNode} from './tree'; /** * Node toggle to expand/collapse the node. */ -@Directive({ - selector: '[cdkTreeNodeToggle]', - host: { - '(click)': '_toggle($event)', - } -}) +@Directive({selector: '[cdkTreeNodeToggle]'}) export class CdkTreeNodeToggle { /** Whether expand/collapse the node recursively. */ @Input('cdkTreeNodeToggleRecursive') @@ -32,6 +25,12 @@ export class CdkTreeNodeToggle { constructor(protected _tree: CdkTree, protected _treeNode: CdkTreeNode) {} + // We have to use a `HostListener` here in order to support both Ivy and ViewEngine. + // In Ivy the `host` bindings will be merged when this class is extended, whereas in + // ViewEngine they're overwritten. + // TODO(crisbeto): we move this back into `host` once Ivy is turned on by default. + // tslint:disable-next-line:no-host-decorator-in-concrete + @HostListener('click', ['$event']) _toggle(event: Event): void { this.recursive ? this._tree.treeControl.toggleDescendants(this._treeNode.data) diff --git a/src/cdk/tree/tree.ts b/src/cdk/tree/tree.ts index 69f97b62470c..521ad527e0cb 100644 --- a/src/cdk/tree/tree.ts +++ b/src/cdk/tree/tree.ts @@ -54,10 +54,14 @@ import { 'role': 'tree', }, encapsulation: ViewEncapsulation.None, - changeDetection: ChangeDetectionStrategy.OnPush + + // The "OnPush" status for the `CdkTree` component is effectively a noop, so we are removing it. + // The view for `CdkTree` consists entirely of templates declared in other views. As they are + // declared elsewhere, they are checked when their declaration points are checked. + // tslint:disable-next-line:validate-decorators + changeDetection: ChangeDetectionStrategy.Default }) -export class CdkTree - implements AfterContentChecked, CollectionViewer, OnDestroy, OnInit { +export class CdkTree implements AfterContentChecked, CollectionViewer, OnDestroy, OnInit { /** Subject that emits when the component has been destroyed. */ private _onDestroy = new Subject(); @@ -299,11 +303,17 @@ export class CdkTreeNode implements FocusableOption, OnDestroy { /** Subject that emits when the component has been destroyed. */ protected _destroyed = new Subject(); + /** Emits when the node's data has changed. */ + _dataChanges = new Subject(); + /** The tree node's data. */ get data(): T { return this._data; } set data(value: T) { - this._data = value; - this._setRoleFromData(); + if (value !== this._data) { + this._data = value; + this._setRoleFromData(); + this._dataChanges.next(); + } } protected _data: T; @@ -333,6 +343,7 @@ export class CdkTreeNode implements FocusableOption, OnDestroy { CdkTreeNode.mostRecentTreeNode = null; } + this._dataChanges.complete(); this._destroyed.next(); this._destroyed.complete(); } diff --git a/src/lib/tree/node.ts b/src/lib/tree/node.ts index 5165d3a74176..086fdcec54da 100644 --- a/src/lib/tree/node.ts +++ b/src/lib/tree/node.ts @@ -6,7 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {CdkNestedTreeNode, CdkTree, CdkTreeNode, CdkTreeNodeDef} from '@angular/cdk/tree'; +import { + CDK_TREE_NODE_OUTLET_NODE, + CdkNestedTreeNode, + CdkTree, + CdkTreeNode, + CdkTreeNodeDef, +} from '@angular/cdk/tree'; import { AfterContentInit, Attribute, @@ -19,12 +25,14 @@ import { QueryList, } from '@angular/core'; import { - CanDisable, CanDisableCtor, + CanDisable, + CanDisableCtor, HasTabIndex, HasTabIndexCtor, mixinDisabled, mixinTabIndex, } from '@angular/material/core'; + import {MatTreeNodeOutlet} from './outlet'; export const _MatTreeNodeMixinBase: HasTabIndexCtor & CanDisableCtor & typeof CdkTreeNode = @@ -90,15 +98,21 @@ export class MatTreeNodeDef extends CdkTreeNodeDef { inputs: ['disabled', 'tabIndex'], providers: [ {provide: CdkNestedTreeNode, useExisting: MatNestedTreeNode}, - {provide: CdkTreeNode, useExisting: MatNestedTreeNode} + {provide: CdkTreeNode, useExisting: MatNestedTreeNode}, + {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: MatNestedTreeNode} ] }) -export class MatNestedTreeNode extends _MatNestedTreeNodeMixinBase - implements AfterContentInit, CanDisable, HasTabIndex, OnDestroy { - +export class MatNestedTreeNode extends _MatNestedTreeNodeMixinBase implements + AfterContentInit, CanDisable, HasTabIndex, OnDestroy { @Input('matNestedTreeNode') node: T; - @ContentChildren(MatTreeNodeOutlet) nodeOutlet: QueryList; + /** The children node placeholder. */ + @ContentChildren(MatTreeNodeOutlet, { + // We need to use `descendants: true`, because Ivy will no longer match + // indirect descendants if it's left as false. + descendants: true + }) + nodeOutlet: QueryList; constructor(protected _elementRef: ElementRef, protected _tree: CdkTree, diff --git a/src/lib/tree/outlet.ts b/src/lib/tree/outlet.ts index 6d01a6bf65e6..e2ad42c7266a 100644 --- a/src/lib/tree/outlet.ts +++ b/src/lib/tree/outlet.ts @@ -5,9 +5,11 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {CdkTreeNodeOutlet} from '@angular/cdk/tree'; +import {CDK_TREE_NODE_OUTLET_NODE, CdkTreeNodeOutlet} from '@angular/cdk/tree'; import { Directive, + Inject, + Optional, ViewContainerRef, } from '@angular/core'; @@ -19,5 +21,7 @@ import { selector: '[matTreeNodeOutlet]' }) export class MatTreeNodeOutlet implements CdkTreeNodeOutlet { - constructor(public viewContainer: ViewContainerRef) {} + constructor( + public viewContainer: ViewContainerRef, + @Inject(CDK_TREE_NODE_OUTLET_NODE) @Optional() public _node?: any) {} } diff --git a/src/lib/tree/toggle.ts b/src/lib/tree/toggle.ts index ba99c3a84add..5c7b9f9a2479 100644 --- a/src/lib/tree/toggle.ts +++ b/src/lib/tree/toggle.ts @@ -14,9 +14,6 @@ import {Directive, Input} from '@angular/core'; */ @Directive({ selector: '[matTreeNodeToggle]', - host: { - '(click)': '_toggle($event)', - }, providers: [{provide: CdkTreeNodeToggle, useExisting: MatTreeNodeToggle}] }) export class MatTreeNodeToggle extends CdkTreeNodeToggle { diff --git a/src/lib/tree/tree.ts b/src/lib/tree/tree.ts index e7a672642494..b3531aa621ac 100644 --- a/src/lib/tree/tree.ts +++ b/src/lib/tree/tree.ts @@ -24,7 +24,9 @@ import {MatTreeNodeOutlet} from './outlet'; }, styleUrls: ['tree.css'], encapsulation: ViewEncapsulation.None, - changeDetection: ChangeDetectionStrategy.OnPush, + // See note on CdkTree for explanation on why this uses the default change detection strategy. + // tslint:disable-next-line:validate-decorators + changeDetection: ChangeDetectionStrategy.Default, providers: [{provide: CdkTree, useExisting: MatTree}] }) export class MatTree extends CdkTree { diff --git a/tools/public_api_guard/cdk/tree.d.ts b/tools/public_api_guard/cdk/tree.d.ts index 628028be6b7a..822d7346021b 100644 --- a/tools/public_api_guard/cdk/tree.d.ts +++ b/tools/public_api_guard/cdk/tree.d.ts @@ -16,6 +16,8 @@ export declare abstract class BaseTreeControl implements TreeControl { toggleDescendants(dataNode: T): void; } +export declare const CDK_TREE_NODE_OUTLET_NODE: InjectionToken<{}>; + export declare class CdkNestedTreeNode extends CdkTreeNode implements AfterContentInit, OnDestroy { protected _children: T[]; protected _differs: IterableDiffers; @@ -53,6 +55,7 @@ export declare class CdkTreeModule { export declare class CdkTreeNode implements FocusableOption, OnDestroy { protected _data: T; + _dataChanges: Subject; protected _destroyed: Subject; protected _elementRef: ElementRef; protected _tree: CdkTree; @@ -75,8 +78,9 @@ export declare class CdkTreeNodeDef { } export declare class CdkTreeNodeOutlet { + _node?: any; viewContainer: ViewContainerRef; - constructor(viewContainer: ViewContainerRef); + constructor(viewContainer: ViewContainerRef, _node?: any); } export declare class CdkTreeNodeOutletContext { @@ -95,7 +99,7 @@ export declare class CdkTreeNodePadding implements OnDestroy { level: number; constructor(_treeNode: CdkTreeNode, _tree: CdkTree, _renderer: Renderer2, _element: ElementRef, _dir: Directionality); _paddingIndent(): string | null; - _setPadding(): void; + _setPadding(forceChange?: boolean): void; ngOnDestroy(): void; } diff --git a/tools/public_api_guard/lib/tree.d.ts b/tools/public_api_guard/lib/tree.d.ts index a7d033eb768e..d3e68ba4538c 100644 --- a/tools/public_api_guard/lib/tree.d.ts +++ b/tools/public_api_guard/lib/tree.d.ts @@ -61,8 +61,9 @@ export declare class MatTreeNodeDef extends CdkTreeNodeDef { } export declare class MatTreeNodeOutlet implements CdkTreeNodeOutlet { + _node?: any; viewContainer: ViewContainerRef; - constructor(viewContainer: ViewContainerRef); + constructor(viewContainer: ViewContainerRef, _node?: any); } export declare class MatTreeNodePadding extends CdkTreeNodePadding {