Skip to content

Commit

Permalink
fix(material/tree): backwards compatible isExpandable
Browse files Browse the repository at this point in the history
Fix backwards compatibility issue for Trees created before
CdkTreeNode#isExpandable was added.

Steps to reproduce.
1. Create a example tree using MatNestedTreeControl. Be sure to *not*
   provide an isExpandable function.
2. Add a parent and child node to example.
3. (App renders a tree with two nodes, parent has chrevron icon on it).
4. Navigate to parent node and press right arrow key on keyboard
5. (Expecting node to expand, but nothing seems to happen).

Work-around: click on the node instead of using keyboard

Change isExpandable logic of CdkTreeNode. Current behavior is that tree
nodes are not expandable unless isExpandable is provided.

With this commit applied, tree nodes that have a tree control will be
expandable by default. Unless isExpandable is provided, trees using
childrenAccessor or levelAccessor are not expandable, but trees using
TreeControl are expandable.
  • Loading branch information
zarend committed Oct 10, 2023
1 parent 4c2069a commit 01dbecd
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 24 deletions.
27 changes: 18 additions & 9 deletions src/cdk/tree/tree-with-tree-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {

import {CollectionViewer, DataSource} from '@angular/cdk/collections';
import {Directionality, Direction} from '@angular/cdk/bidi';
import {createKeyboardEvent} from '@angular/cdk/testing/testbed/fake-events';
import {combineLatest, BehaviorSubject, Observable} from 'rxjs';
import {map} from 'rxjs/operators';

Expand All @@ -27,6 +28,7 @@ import {FlatTreeControl} from './control/flat-tree-control';
import {NestedTreeControl} from './control/nested-tree-control';
import {CdkTreeModule, CdkTreeNodePadding} from './index';
import {CdkTree, CdkTreeNode} from './tree';
import {LEFT_ARROW, RIGHT_ARROW} from '../keycodes';

describe('CdkTree', () => {
/** Represents an indent for expectNestedTreeToMatch */
Expand Down Expand Up @@ -758,7 +760,7 @@ describe('CdkTree', () => {
it('with the right aria-expanded attrs', () => {
expect(getNodeAttributes(getNodes(treeElement), 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, null, null]);
.toEqual(['false', 'false', 'false']);

component.toggleRecursively = false;
let data = dataSource.data;
Expand All @@ -773,10 +775,10 @@ describe('CdkTree', () => {
// in DOM unless the parent node is expanded.
expect(getNodeAttributes(getNodes(treeElement), 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, 'true', 'false', null]);
.toEqual(['false', 'true', 'false', 'false']);
});

it('should expand/collapse the node multiple times', () => {
it('should expand/collapse the node multiple times using keyboard', () => {
component.toggleRecursively = false;
let data = dataSource.data;
const child = dataSource.addChild(data[1], false);
Expand All @@ -793,7 +795,10 @@ describe('CdkTree', () => {

fixture.detectChanges();

(getNodes(treeElement)[1] as HTMLElement).click();
let node = getNodes(treeElement)[1] as HTMLElement;

node.focus();
node.dispatchEvent(createKeyboardEvent('keydown', RIGHT_ARROW));
fixture.detectChanges();

expect(component.treeControl.expansionModel.selected.length)
Expand All @@ -807,7 +812,9 @@ describe('CdkTree', () => {
[`topping_3 - cheese_3 + base_3`],
);

(getNodes(treeElement)[1] as HTMLElement).click();
node = getNodes(treeElement)[1] as HTMLElement;
node.focus();
node.dispatchEvent(createKeyboardEvent('keydown', LEFT_ARROW));
fixture.detectChanges();

expectNestedTreeToMatch(
Expand All @@ -820,7 +827,9 @@ describe('CdkTree', () => {
.withContext(`Expect node collapsed`)
.toBe(0);

(getNodes(treeElement)[1] as HTMLElement).click();
node = getNodes(treeElement)[1] as HTMLElement;
node.focus();
node.dispatchEvent(createKeyboardEvent('keydown', RIGHT_ARROW));
fixture.detectChanges();

expect(component.treeControl.expansionModel.selected.length)
Expand Down Expand Up @@ -1585,9 +1594,9 @@ class NestedCdkTreeAppWithToggle {

getChildren = (node: TestData) => node.observableChildren;

treeControl: TreeControl<TestData> = new NestedTreeControl(this.getChildren, {
isExpandable: node => node.children.length > 0,
});
isExpandable?: (node: TestData) => boolean;

treeControl: TreeControl<TestData> = new NestedTreeControl(this.getChildren);
dataSource: FakeDataSource | null = new FakeDataSource(this.treeControl);

@ViewChild(CdkTree) tree: CdkTree<TestData>;
Expand Down
20 changes: 12 additions & 8 deletions src/cdk/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1175,8 +1175,14 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI

/** Determines if the tree node is expandable. */
_isExpandable(): boolean {
if (typeof this._tree.treeControl?.isExpandable === 'function') {
return this._tree.treeControl.isExpandable(this._data);
if (this._tree.treeControl) {
if (typeof this._tree.treeControl?.isExpandable === 'function') {
return this._tree.treeControl.isExpandable(this._data);
}

// For compatibility with trees created using TreeControl before we added
// CdkTreeNode#isExpandable.
return true;
}
return this._inputIsExpandable;
}
Expand Down Expand Up @@ -1260,18 +1266,16 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI

/** Collapses this data node. Implemented for TreeKeyManagerItem. */
collapse(): void {
if (!this._isExpandable()) {
return;
if (this.isExpandable) {
this._tree.collapse(this._data);
}
this._tree.collapse(this._data);
}

/** Expands this data node. Implemented for TreeKeyManagerItem. */
expand(): void {
if (!this._isExpandable()) {
return;
if (this.isExpandable) {
this._tree.expand(this._data);
}
this._tree.expand(this._data);
}

_setTabFocusable() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
<!-- This is the tree node template for expandable nodes -->
<mat-nested-tree-node
*matTreeNodeDef="let node; when: hasChild"
matTreeNodeToggle
isExpandable>
matTreeNodeToggle>
<div class="mat-tree-node">
<button mat-icon-button matTreeNodeToggle
[attr.aria-label]="'Toggle ' + node.name">
Expand Down
10 changes: 5 additions & 5 deletions src/material/tree/tree-using-tree-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ describe('MatTree', () => {
it('with the right aria-expanded attrs', () => {
expect(getNodeAttributes(getNodes(treeElement), 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, null, null]);
.toEqual(['false', 'false', 'false']);

component.toggleRecursively = false;
const data = underlyingDataSource.data;
Expand All @@ -456,7 +456,7 @@ describe('MatTree', () => {
// in DOM unless the parent node is expanded.
expect(getNodeAttributes(getNodes(treeElement), 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, 'true', 'false', null]);
.toEqual(['false', 'true', 'false', 'false']);
});

it('should expand/collapse the node', () => {
Expand Down Expand Up @@ -656,7 +656,7 @@ describe('MatTree', () => {
it('sets aria attributes for tree nodes', () => {
expect(getNodeAttributes(nodes, 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, 'false', 'false', null, null, null]);
.toEqual(['false', 'false', 'false', 'false', 'false', 'false']);
expect(getNodeAttributes(nodes, 'aria-level'))
.withContext('aria-level attributes')
.toEqual(['1', '1', '2', '3', '3', '1']);
Expand All @@ -673,13 +673,13 @@ describe('MatTree', () => {
fixture.detectChanges();
expect(getNodeAttributes(nodes, 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, 'true', 'false', null, null, null]);
.toEqual(['false', 'true', 'false', 'false', 'false', 'false']);

tree.collapse(underlyingDataSource.data[1]);
fixture.detectChanges();
expect(getNodeAttributes(nodes, 'aria-expanded'))
.withContext('aria-expanded attributes')
.toEqual([null, 'false', 'false', null, null, null]);
.toEqual(['false', 'false', 'false', 'false', 'false', 'false']);
});
});
});
Expand Down

0 comments on commit 01dbecd

Please sign in to comment.