From ab5cb56f63a70fb663a5d22a99fe1a7702efc709 Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Wed, 6 Dec 2023 09:21:26 -0800 Subject: [PATCH] refactor(material/tree): docs updates and rename variables (#28238) Make miscelaneous fixes to documentation. Rename variables. Responding to PR feedback. * remove comment about API contract of CdkTreeNodeToggle * rename CdkTree#_groups to CdkTree#_ariaSets * move documentation about default key manager configuration from TreeKeyManagerStrategy interface to TreeKeyManager class. * add JSDoc style comments for NoopTreeKeyManager --- .../a11y/key-manager/noop-tree-key-manager.ts | 21 +++++++++ .../key-manager/tree-key-manager-strategy.ts | 11 ++--- src/cdk/a11y/key-manager/tree-key-manager.ts | 29 ++++++++---- src/cdk/tree/toggle.ts | 3 -- src/cdk/tree/tree.ts | 46 +++++++++---------- tools/public_api_guard/cdk/a11y.md | 2 +- 6 files changed, 69 insertions(+), 43 deletions(-) diff --git a/src/cdk/a11y/key-manager/noop-tree-key-manager.ts b/src/cdk/a11y/key-manager/noop-tree-key-manager.ts index 3ad1e0253141..5f9702b4b3d1 100644 --- a/src/cdk/a11y/key-manager/noop-tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/noop-tree-key-manager.ts @@ -22,6 +22,13 @@ import { /** * @docs-private * + * Opt-out of Tree of key manager behavior. + * + * When provided, Tree has same focus management behavior as before TreeKeyManager was introduced. + * - Tree does not respond to keyboard interaction + * - Tree node allows tabindex to be set by Input binding + * - Tree node allows tabindex to be set by attribute binding + * * @deprecated NoopTreeKeyManager deprecated. Use TreeKeyManager or inject a * TreeKeyManagerStrategy instead. To be removed in a future version. * @@ -62,6 +69,13 @@ export class NoopTreeKeyManager implements TreeKey /** * @docs-private * + * Opt-out of Tree of key manager behavior. + * + * When provided, Tree has same focus management behavior as before TreeKeyManager was introduced. + * - Tree does not respond to keyboard interaction + * - Tree node allows tabindex to be set by Input binding + * - Tree node allows tabindex to be set by attribute binding + * * @deprecated NoopTreeKeyManager deprecated. Use TreeKeyManager or inject a * TreeKeyManagerStrategy instead. To be removed in a future version. * @@ -76,6 +90,13 @@ export function NOOP_TREE_KEY_MANAGER_FACTORY< /** * @docs-private * + * Opt-out of Tree of key manager behavior. + * + * When provided, Tree has same focus management behavior as before TreeKeyManager was introduced. + * - Tree does not respond to keyboard interaction + * - Tree node allows tabindex to be set by Input binding + * - Tree node allows tabindex to be set by attribute binding + * * @deprecated NoopTreeKeyManager deprecated. Use TreeKeyManager or inject a * TreeKeyManagerStrategy instead. To be removed in a future version. * diff --git a/src/cdk/a11y/key-manager/tree-key-manager-strategy.ts b/src/cdk/a11y/key-manager/tree-key-manager-strategy.ts index 3327f648f2a5..084f031462ae 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager-strategy.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager-strategy.ts @@ -52,21 +52,20 @@ export interface TreeKeyManagerItem { export interface TreeKeyManagerOptions { /** * If true, then the key manager will call `activate` in addition to calling `focus` when a - * particular item is focused. By default, this is false. + * particular item is focused. */ - activationFollowsFocus?: boolean; + shouldActivationFollowFocus?: boolean; /** * The direction in which the tree items are laid out horizontally. This influences which key - * will be interpreted as expand or collapse. Defaults to 'ltr'. + * will be interpreted as expand or collapse. */ horizontalOrientation?: 'rtl' | 'ltr'; /** - * Sets the predicate function that determines which items should be skipped by the tree key - * manager. By default, disabled items are skipped. + * If provided, navigation "skips" over items that pass the given predicate. * - * If the item is to be skipped, this function should return false. + * If the item is to be skipped, predicate function should return false. */ skipPredicate?: (item: T) => boolean; diff --git a/src/cdk/a11y/key-manager/tree-key-manager.ts b/src/cdk/a11y/key-manager/tree-key-manager.ts index 07d623e00feb..c3d1fd683e0f 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.ts @@ -43,8 +43,8 @@ function coerceObservable(data: T | Observable): Observable { export class TreeKeyManager implements TreeKeyManagerStrategy { private _activeItemIndex = -1; private _activeItem: T | null = null; - private _activationFollowsFocus = false; - private _horizontal: 'ltr' | 'rtl' = 'ltr'; + private _shouldActivationFollowFocus = false; + private _horizontalOrientation: 'ltr' | 'rtl' = 'ltr'; // Keep tree items focusable when disabled. Align with // https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols. @@ -85,6 +85,14 @@ export class TreeKeyManager implements TreeKeyMana this._hasInitialFocused = true; } + /** + * + * @param items List of TreeKeyManager options. Can be synchronous or asynchronous. + * @param config Optional configuration options. By default, use 'ltr' horizontal orientation. By + * default, do not skip any nodes. By default, key manager only calls `focus` method when items + * are focused and does not call `activate`. If `typeaheadDefaultInterval` is `true`, use a + * default interval of 200ms. + */ constructor(items: Observable | QueryList | T[], config: TreeKeyManagerOptions) { // We allow for the items to be an array or Observable because, in some cases, the consumer may // not have access to a QueryList of the items they want to manage (e.g. when the @@ -109,11 +117,11 @@ export class TreeKeyManager implements TreeKeyMana this._initialFocus(); } - if (typeof config.activationFollowsFocus === 'boolean') { - this._activationFollowsFocus = config.activationFollowsFocus; + if (typeof config.shouldActivationFollowFocus === 'boolean') { + this._shouldActivationFollowFocus = config.shouldActivationFollowFocus; } if (config.horizontalOrientation) { - this._horizontal = config.horizontalOrientation; + this._horizontalOrientation = config.horizontalOrientation; } if (config.skipPredicate) { this._skipPredicateFn = config.skipPredicate; @@ -157,11 +165,15 @@ export class TreeKeyManager implements TreeKeyMana break; case RIGHT_ARROW: - this._horizontal === 'rtl' ? this._collapseCurrentItem() : this._expandCurrentItem(); + this._horizontalOrientation === 'rtl' + ? this._collapseCurrentItem() + : this._expandCurrentItem(); break; case LEFT_ARROW: - this._horizontal === 'rtl' ? this._expandCurrentItem() : this._collapseCurrentItem(); + this._horizontalOrientation === 'rtl' + ? this._expandCurrentItem() + : this._collapseCurrentItem(); break; case HOME: @@ -264,11 +276,10 @@ export class TreeKeyManager implements TreeKeyMana previousActiveItem?.unfocus(); if (options.emitChangeEvent) { - // Emit to `change` stream as required by TreeKeyManagerStrategy interface. this.change.next(this._activeItem); } - if (this._activationFollowsFocus) { + if (this._shouldActivationFollowFocus) { this._activateCurrentItem(); } } diff --git a/src/cdk/tree/toggle.ts b/src/cdk/tree/toggle.ts index 074261bb0f42..a10e17f80007 100644 --- a/src/cdk/tree/toggle.ts +++ b/src/cdk/tree/toggle.ts @@ -12,9 +12,6 @@ import {CdkTree, CdkTreeNode} from './tree'; /** * Node toggle to expand and collapse the node. - * - * CdkTreeNodeToggle is intended only to be used on native button elements, elements with button role, - * or elements with treeitem role. */ @Directive({ selector: '[cdkTreeNodeToggle]', diff --git a/src/cdk/tree/tree.ts b/src/cdk/tree/tree.ts index 81916d4392cc..4631a4a16d2b 100644 --- a/src/cdk/tree/tree.ts +++ b/src/cdk/tree/tree.ts @@ -132,16 +132,14 @@ export class CdkTree private _parents: Map = new Map(); /** - * The internal node groupings for each node; we use this to determine where - * a particular node is within each group. This allows us to compute the - * correct aria attribute values. + * Nodes grouped into each set, which is a list of nodes displayed together in the DOM. * - * The structure of this is that: - * - the outer index is the level - * - the inner index is the parent node for this particular group. If there is no parent node, we - * use `null`. + * Lookup key is the parent of a set. Root nodes have key of null. + * + * Values is a 'set' of tree nodes. Each tree node maps to a treeitem element. Sets are in the + * order that it is rendered. Each set maps directly to aria-posinset and aria-setsize attributes. */ - private _groups: Map = new Map(); + private _ariaSets: Map = new Map(); /** * Provides a stream containing the latest data array to render. Influenced by the tree's @@ -500,10 +498,10 @@ export class CdkTree this.insertNode(data[currentIndex!], currentIndex!, viewContainer, parentData); } else if (currentIndex == null) { viewContainer.remove(adjustedPreviousIndex!); - const group = this._getNodeGroup(item.item); + const set = this._getAriaSet(item.item); const key = this._getExpansionKey(item.item); - group.splice( - group.findIndex(groupItem => this._getExpansionKey(groupItem) === key), + set.splice( + set.findIndex(groupItem => this._getExpansionKey(groupItem) === key), 1, ); } else { @@ -784,8 +782,8 @@ export class CdkTree * This is intended to be used for `aria-setsize`. */ _getSetSize(dataNode: T) { - const group = this._getNodeGroup(dataNode); - return group.length; + const set = this._getAriaSet(dataNode); + return set.length; } /** @@ -794,9 +792,9 @@ export class CdkTree * This is intended to be used for `aria-posinset`. */ _getPositionInSet(dataNode: T) { - const group = this._getNodeGroup(dataNode); + const set = this._getAriaSet(dataNode); const key = this._getExpansionKey(dataNode); - return group.findIndex(node => this._getExpansionKey(node) === key) + 1; + return set.findIndex(node => this._getExpansionKey(node) === key) + 1; } /** Given a CdkTreeNode, gets the node that renders that node's parent's data. */ @@ -902,12 +900,12 @@ export class CdkTree return this.expansionKey?.(dataNode) ?? (dataNode as unknown as K); } - private _getNodeGroup(node: T) { + private _getAriaSet(node: T) { const key = this._getExpansionKey(node); const parent = this._parents.get(key); const parentKey = parent ? this._getExpansionKey(parent) : null; - const group = this._groups.get(parentKey); - return group ?? [node]; + const set = this._ariaSets.get(parentKey); + return set ?? [node]; } /** @@ -963,7 +961,7 @@ export class CdkTree children.pipe( take(1), tap(childNodes => { - this._groups.set(parentKey, [...(childNodes ?? [])]); + this._ariaSets.set(parentKey, [...(childNodes ?? [])]); for (const child of childNodes ?? []) { const childKey = this._getExpansionKey(child); this._parents.set(childKey, node); @@ -1006,7 +1004,7 @@ export class CdkTree // nested. if (this.childrenAccessor && nodeType === 'flat') { // This flattens children into a single array. - this._groups.set(null, [...nodes]); + this._ariaSets.set(null, [...nodes]); return this._flattenNestedNodesWithExpansion(nodes).pipe( map(flattenedNodes => ({ renderNodes: flattenedNodes, @@ -1039,7 +1037,7 @@ export class CdkTree } else { // For nested nodes, we still need to perform the node flattening in order // to maintain our caches for various tree operations. - this._groups.set(null, [...nodes]); + this._ariaSets.set(null, [...nodes]); return this._flattenNestedNodesWithExpansion(nodes).pipe( map(flattenedNodes => ({ renderNodes: nodes, @@ -1065,7 +1063,7 @@ export class CdkTree } this._parents.clear(); - this._groups.clear(); + this._ariaSets.clear(); for (let index = 0; index < flattenedNodes.length; index++) { const dataNode = flattenedNodes[index]; @@ -1075,9 +1073,9 @@ export class CdkTree this._parents.set(key, parent); const parentKey = parent ? this._getExpansionKey(parent) : null; - const group = this._groups.get(parentKey) ?? []; + const group = this._ariaSets.get(parentKey) ?? []; group.splice(index, 0, dataNode); - this._groups.set(parentKey, group); + this._ariaSets.set(parentKey, group); } } } diff --git a/tools/public_api_guard/cdk/a11y.md b/tools/public_api_guard/cdk/a11y.md index 81b7568e1456..997d5d316e0e 100644 --- a/tools/public_api_guard/cdk/a11y.md +++ b/tools/public_api_guard/cdk/a11y.md @@ -505,8 +505,8 @@ export interface TreeKeyManagerItem { // @public export interface TreeKeyManagerOptions { - activationFollowsFocus?: boolean; horizontalOrientation?: 'rtl' | 'ltr'; + shouldActivationFollowFocus?: boolean; skipPredicate?: (item: T) => boolean; trackBy?: (treeItem: T) => unknown; typeAheadDebounceInterval?: true | number;