Skip to content

Commit

Permalink
fix(cdk/tree): don't focus on click, corrected updating aria-sets
Browse files Browse the repository at this point in the history
  • Loading branch information
BobobUnicorn committed Feb 2, 2024
1 parent f160dc1 commit 9a633b7
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/cdk/tree/tree-with-tree-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,7 @@ describe('CdkTree', () => {
it('maintains tabindex when component is blurred', () => {
// activate the second child by clicking on it
nodes[1].click();
nodes[1].focus();
fixture.detectChanges();

expect(document.activeElement).toBe(nodes[1]);
Expand Down
1 change: 1 addition & 0 deletions src/cdk/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,7 @@ describe('CdkTree', () => {
it('maintains tabindex when component is blurred', () => {
// activate the second child by clicking on it
nodes[1].click();
nodes[1].focus();
fixture.detectChanges();

expect(document.activeElement).toBe(nodes[1]);
Expand Down
32 changes: 23 additions & 9 deletions src/cdk/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,9 @@ export class CdkTree<T, K = T>
this._dataSubscription = null;
}

this._keyManager.destroy();
// In certain tests, the tree might be destroyed before this is initialized
// in `ngAfterContentInit`.
this._keyManager?.destroy();
}

ngOnInit() {
Expand Down Expand Up @@ -520,12 +522,6 @@ export class CdkTree<T, K = T>
this.insertNode(data[currentIndex!], currentIndex!, viewContainer, parentData);
} else if (currentIndex == null) {
viewContainer.remove(adjustedPreviousIndex!);
const set = this._getAriaSet(item.item);
const key = this._getExpansionKey(item.item);
set.splice(
set.findIndex(groupItem => this._getExpansionKey(groupItem) === key),
1,
);
} else {
const view = viewContainer.get(adjustedPreviousIndex!);
viewContainer.move(view!, currentIndex);
Expand Down Expand Up @@ -1116,7 +1112,7 @@ export class CdkTree<T, K = T>
'[attr.aria-setsize]': '_getSetSize()',
'[tabindex]': '_tabindex',
'role': 'treeitem',
'(click)': '_focusItem()',
'(click)': '_setActiveItem()',
'(focus)': '_focusItem()',
},
standalone: true,
Expand Down Expand Up @@ -1194,6 +1190,13 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
readonly _dataChanges = new Subject<void>();

private _inputIsExpandable: boolean = false;
/**
* Flag used to determine whether or not we should be focusing the actual element based on
* some user interaction (click or focus). On click, we don't forcibly focus the element
* since the click could trigger some other component that wants to grab its own focus
* (e.g. menu, dialog).
*/
private _shouldFocus = true;
private _parentNodeAriaLevel: number;

/** The tree node's data. */
Expand Down Expand Up @@ -1295,7 +1298,9 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
/** Focuses this data node. Implemented for TreeKeyManagerItem. */
focus(): void {
this._tabindex = 0;
this._elementRef.nativeElement.focus();
if (this._shouldFocus) {
this._elementRef.nativeElement.focus();
}

this._changeDetectorRef.markForCheck();
}
Expand Down Expand Up @@ -1336,6 +1341,15 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
this._tree._keyManager.focusItem(this);
}

_setActiveItem() {
if (this.isDisabled) {
return;
}
this._shouldFocus = false;
this._tree._keyManager.focusItem(this);
this._shouldFocus = true;
}

_emitExpansionState(expanded: boolean) {
this.expandedChange.emit(expanded);
}
Expand Down
1 change: 1 addition & 0 deletions src/material/tree/tree-using-tree-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ describe('MatTree', () => {
it('maintains tabindex when component is blurred', () => {
// activate the second child by clicking on it
nodes[1].click();
nodes[1].focus();
fixture.detectChanges();

expect(document.activeElement).toBe(nodes[1]);
Expand Down
1 change: 1 addition & 0 deletions src/material/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ describe('MatTree', () => {
it('maintains tabindex when component is blurred', () => {
// activate the second child by clicking on it
nodes[1].click();
nodes[1].focus();
fixture.detectChanges();

expect(nodes.map(x => x.getAttribute('tabindex')).join(', ')).toEqual(
Expand Down

0 comments on commit 9a633b7

Please sign in to comment.