Skip to content

Commit

Permalink
fix(cdk/tree): CdkTreeNodeToggle focuses node when toggling it
Browse files Browse the repository at this point in the history
Fix focus behavior of CdkTreeNodeToggle. When toggling the expanded or
collapsed state of a node, focus that node. Fix issue where end user
cannot tab into tree component when collaping the parent of the active
node.

Before this commit is applied, there is a bug where end user cannot tab
into the tree.

Reproduction steps
1. Active a tree node
2. (focus state renders)
3. Using mouse, collapse parent of node from step 1.
4. (tree node collapses)
5. Press Tab, then shift + tab
6. (item before tree is focused. Can tab into the tree)

With this commit applied, above issue is no longer expected to
reproduce.

This commit message is only for reviewers of this PR and can be deleted
when landing this change in main.
  • Loading branch information
zarend committed Aug 29, 2023
1 parent f0d0d1a commit e8377b8
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
6 changes: 6 additions & 0 deletions src/cdk/tree/toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,17 @@ export class CdkTreeNodeToggle<T, K = T> {

constructor(protected _tree: CdkTree<T, K>, protected _treeNode: CdkTreeNode<T, K>) {}

// Toggle the expanded or collapsed state of this node.
//
// Focus this node with expanding or collapsing it. This ensures that the active node will always
// be visible when expanding and collapsing.
_toggle(event: Event): void {
this.recursive
? this._tree.toggleDescendants(this._treeNode.data)
: this._tree.toggle(this._treeNode.data);

this._tree._keyManager.focusItem(this._treeNode);

event.stopPropagation();
}

Expand Down
39 changes: 36 additions & 3 deletions src/cdk/tree/tree-redesign.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,28 @@ describe('CdkTree redesign', () => {
.toBe(0);
});

it('should focus a node when collapsing it', () => {
// Create a tree with two nodes. A parent node and its child.
dataSource.clear();
const parent = dataSource.addData();
dataSource.addChild(parent);

component.tree.expandAll();
fixture.detectChanges();

// focus the child node
getNodes(treeElement)[1].click();
fixture.detectChanges();

// collapse the parent node
getNodes(treeElement)[0].click();
fixture.detectChanges();

expect(getNodes(treeElement).map(x => x.getAttribute('tabindex')))
.withContext(`Expecting parent node to be focused since it was collapsed.`)
.toEqual(['0', '-1']);
});

it('should expand/collapse the node recursively', () => {
expect(dataSource.data.length).toBe(3);

Expand Down Expand Up @@ -1312,22 +1334,33 @@ class FakeDataSource extends DataSource<TestData> {
return child;
}

addData(level: number = 1) {
addData(level: number = 1): TestData {
const nextIndex = ++this.dataIndex;

let copiedData = this.data.slice();
copiedData.push(
new TestData(`topping_${nextIndex}`, `cheese_${nextIndex}`, `base_${nextIndex}`, level),
const newData = new TestData(
`topping_${nextIndex}`,
`cheese_${nextIndex}`,
`base_${nextIndex}`,
level,
);
copiedData.push(newData);

this.data = copiedData;

return newData;
}

getRecursiveData(nodes: TestData[] = this._dataChange.getValue()): TestData[] {
return [
...new Set(nodes.flatMap(parent => [parent, ...this.getRecursiveData(parent.children)])),
];
}

clear() {
this.data = [];
this.dataIndex = 0;
}
}

function getNodes(treeElement: Element): HTMLElement[] {
Expand Down

0 comments on commit e8377b8

Please sign in to comment.