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(cdk/tree): assorted bug fixes #28305

Merged
merged 6 commits into from May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
34 changes: 28 additions & 6 deletions src/cdk/tree/tree.ts
Expand Up @@ -23,6 +23,7 @@ import {
import {
AfterContentChecked,
AfterContentInit,
AfterViewInit,
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
Expand Down Expand Up @@ -111,7 +112,13 @@ type RenderingData<T> =
imports: [CdkTreeNodeOutlet],
})
export class CdkTree<T, K = T>
implements AfterContentChecked, AfterContentInit, CollectionViewer, OnDestroy, OnInit
implements
AfterContentChecked,
AfterContentInit,
AfterViewInit,
CollectionViewer,
OnDestroy,
OnInit
{
/** Subject that emits when the component has been destroyed. */
private readonly _onDestroy = new Subject<void>();
Expand Down Expand Up @@ -248,6 +255,7 @@ export class CdkTree<T, K = T>

/** The key manager for this tree. Handles focus and activation based on user keyboard input. */
_keyManager: TreeKeyManagerStrategy<CdkTreeNode<T, K>>;
private _viewInit = false;

constructor(
private _differs: IterableDiffers,
Expand Down Expand Up @@ -288,6 +296,10 @@ export class CdkTree<T, K = T>
this._initializeDataDiffer();
}

ngAfterViewInit() {
this._viewInit = true;
}

private _updateDefaultNodeDefinition() {
const defaultNodeDefs = this._nodeDefs.filter(def => !def.when);
if (defaultNodeDefs.length > 1 && (typeof ngDevMode === 'undefined' || ngDevMode)) {
Expand Down Expand Up @@ -449,7 +461,9 @@ export class CdkTree<T, K = T>
}

private _initializeDataDiffer() {
this._dataDiffer = this._differs.find([]).create(this.trackBy);
// Provide a default trackBy based on `_getExpansionKey` if one isn't provided.
const trackBy = this.trackBy ?? ((_index: number, item: T) => this._getExpansionKey(item));
this._dataDiffer = this._differs.find([]).create(trackBy);
}

private _checkTreeControlUsage() {
Expand Down Expand Up @@ -484,11 +498,19 @@ export class CdkTree<T, K = T>
parentData?: T,
) {
const changes = dataDiffer.diff(data);
if (!changes) {

// Some tree consumers expect change detection to propagate to nodes
// even when the array itself hasn't changed; we explicitly detect changes
// anyways in order for nodes to update their data.
//
// However, if change detection is called while the component's view is
// still initing, then the order of child views initing will be incorrect;
// to prevent this, we only exit early if the view hasn't initialized yet.
if (!changes && !this._viewInit) {
return;
}

changes.forEachOperation(
changes?.forEachOperation(
(
item: IterableChangeRecord<T>,
adjustedPreviousIndex: number | null,
Expand Down Expand Up @@ -682,12 +704,12 @@ export class CdkTree<T, K = T>

/** Level accessor, used for compatibility between the old Tree and new Tree */
_getLevelAccessor() {
return this.treeControl?.getLevel ?? this.levelAccessor;
return this.treeControl?.getLevel?.bind(this.treeControl) ?? this.levelAccessor;
}

/** Children accessor, used for compatibility between the old Tree and new Tree */
_getChildrenAccessor() {
return this.treeControl?.getChildren ?? this.childrenAccessor;
return this.treeControl?.getChildren?.bind(this.treeControl) ?? this.childrenAccessor;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/material/tree/testing/node-harness.ts
Expand Up @@ -35,6 +35,11 @@ export class MatTreeNodeHarness extends ContentContainerComponentHarness<string>
return coerceBooleanProperty(await (await this.host()).getAttribute('aria-expanded'));
}

/** Whether the tree node is expandable. */
async isExpandable(): Promise<boolean> {
return (await (await this.host()).getAttribute('aria-expanded')) !== null;
}

/** Whether the tree node is disabled. */
async isDisabled(): Promise<boolean> {
return coerceBooleanProperty(await (await this.host()).getProperty('aria-disabled'));
Expand Down
3 changes: 2 additions & 1 deletion src/material/tree/tree.spec.ts
Expand Up @@ -604,11 +604,12 @@ describe('MatTree', () => {
});

it('ignores clicks on disabled items', () => {
underlyingDataSource.data[0].isDisabled = true;
underlyingDataSource.data[1].isDisabled = true;
fixture.detectChanges();

// attempt to click on the first child
nodes[1].click();
fixture.detectChanges();

expect(nodes.map(x => x.getAttribute('tabindex')).join(', ')).toEqual(
'0, -1, -1, -1, -1, -1',
Expand Down
5 changes: 4 additions & 1 deletion tools/public_api_guard/cdk/tree.md
Expand Up @@ -6,6 +6,7 @@

import { AfterContentChecked } from '@angular/core';
import { AfterContentInit } from '@angular/core';
import { AfterViewInit } from '@angular/core';
import { BehaviorSubject } from 'rxjs';
import { ChangeDetectorRef } from '@angular/core';
import { CollectionViewer } from '@angular/cdk/collections';
Expand Down Expand Up @@ -76,7 +77,7 @@ export class CdkNestedTreeNode<T, K = T> extends CdkTreeNode<T, K> implements Af
}

// @public
export class CdkTree<T, K = T> implements AfterContentChecked, AfterContentInit, CollectionViewer, OnDestroy, OnInit {
export class CdkTree<T, K = T> implements AfterContentChecked, AfterContentInit, AfterViewInit, CollectionViewer, OnDestroy, OnInit {
constructor(_differs: IterableDiffers, _changeDetectorRef: ChangeDetectorRef, _dir: Directionality);
childrenAccessor?: (dataNode: T) => T[] | Observable<T[]>;
collapse(dataNode: T): void;
Expand Down Expand Up @@ -106,6 +107,8 @@ export class CdkTree<T, K = T> implements AfterContentChecked, AfterContentInit,
// (undocumented)
ngAfterContentInit(): void;
// (undocumented)
ngAfterViewInit(): void;
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
ngOnInit(): void;
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/tree-testing.md
Expand Up @@ -27,6 +27,7 @@ export class MatTreeNodeHarness extends ContentContainerComponentHarness<string>
getText(): Promise<string>;
static hostSelector: string;
isDisabled(): Promise<boolean>;
isExpandable(): Promise<boolean>;
isExpanded(): Promise<boolean>;
toggle(): Promise<void>;
// (undocumented)
Expand Down