Skip to content

Commit

Permalink
fix(cdk/tree): only handle keyboard events directly from the node
Browse files Browse the repository at this point in the history
Currently the CDK tree handle any keyboard event coming from a descendant. This problematic if there are interactive elements like inputs inside the tree, because their default behavior will be prevented.

This change switches to only handling events coming either directly from the tree or directly from a tree node.

Fixes #29828.
  • Loading branch information
crisbeto committed Oct 30, 2024
1 parent a98c886 commit 5d1bc76
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
22 changes: 22 additions & 0 deletions src/cdk/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,28 @@ describe('CdkTree', () => {
.withContext(`Expect node collapsed`)
.toBe(0);
});

it('should not handle events coming from a descendant of a node', () => {
expect(dataSource.data.length).toBe(3);

expect(getExpandedNodes(component.dataSource?.getRecursiveData(), component.tree).length)
.withContext('Expect no expanded node on init')
.toBe(0);

const node = getNodes(treeElement)[2] as HTMLElement;
const input = document.createElement('input');
node.appendChild(input);

const event = createKeyboardEvent('keydown', undefined, 'ArrowRight');
spyOn(event, 'preventDefault').and.callThrough();
input.dispatchEvent(event);
fixture.detectChanges();

expect(getExpandedNodes(component.dataSource?.getRecursiveData(), component.tree).length)
.withContext('Expect no expanded node after event')
.toBe(0);
expect(event.preventDefault).not.toHaveBeenCalled();
});
});

describe('with when node template', () => {
Expand Down
19 changes: 16 additions & 3 deletions src/cdk/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export class CdkTree<T, K = T>
OnDestroy,
OnInit
{
private _elementRef = inject(ElementRef);
private _dir = inject(Directionality);

/** Subject that emits when the component has been destroyed. */
Expand Down Expand Up @@ -889,8 +890,20 @@ export class CdkTree<T, K = T>
}

/** `keydown` event handler; this just passes the event to the `TreeKeyManager`. */
_sendKeydownToKeyManager(event: KeyboardEvent) {
this._keyManager.onKeydown(event);
protected _sendKeydownToKeyManager(event: KeyboardEvent): void {
// Only handle events directly on the tree or directly on one of the nodes, otherwise
// we risk interfering with events in the projected content (see #29828).
if (event.target === this._elementRef.nativeElement) {
this._keyManager.onKeydown(event);
} else {
const nodes = this._nodes.getValue();
for (const [, node] of nodes) {
if (event.target === node._elementRef.nativeElement) {
this._keyManager.onKeydown(event);
break;
}
}
}
}

/** Gets all nested descendants of a given node. */
Expand Down Expand Up @@ -1341,7 +1354,7 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
private _changeDetectorRef = inject(ChangeDetectorRef);

constructor(
protected _elementRef: ElementRef<HTMLElement>,
public _elementRef: ElementRef<HTMLElement>,
protected _tree: CdkTree<T, K>,
) {
CdkTreeNode.mostRecentTreeNode = this as CdkTreeNode<T, K>;
Expand Down
4 changes: 2 additions & 2 deletions tools/public_api_guard/cdk/tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class CdkTree<T, K = T> implements AfterContentChecked, AfterContentInit,
_nodeOutlet: CdkTreeNodeOutlet;
_registerNode(node: CdkTreeNode<T, K>): void;
renderNodeChanges(data: readonly T[], dataDiffer?: IterableDiffer<T>, viewContainer?: ViewContainerRef, parentData?: T): void;
_sendKeydownToKeyManager(event: KeyboardEvent): void;
protected _sendKeydownToKeyManager(event: KeyboardEvent): void;
_setNodeTypeIfUnset(nodeType: 'flat' | 'nested'): void;
toggle(dataNode: T): void;
toggleDescendants(dataNode: T): void;
Expand Down Expand Up @@ -160,7 +160,7 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
readonly _dataChanges: Subject<void>;
protected readonly _destroyed: Subject<void>;
// (undocumented)
protected _elementRef: ElementRef<HTMLElement>;
_elementRef: ElementRef<HTMLElement>;
// (undocumented)
_emitExpansionState(expanded: boolean): void;
expand(): void;
Expand Down

0 comments on commit 5d1bc76

Please sign in to comment.