From aeb6f893002e18927cb4c4d9570c680bb3527900 Mon Sep 17 00:00:00 2001 From: Escher Wenberg Date: Thu, 6 Aug 2020 10:13:13 -0700 Subject: [PATCH] fix(material/tree): Apply aria-level to all nodes (#17818) * fix(material/tree): Apply aria-level to all nodes Previously, only leaf nodes had aria-level applied. This is an incremental change since this is an unfamiliar codebase for me. The main benefit it will have on its own is that it will allow anyone doing custom dom manipulation to know what level the node is on. Otherwise by itself there is no change in how NVDA reads nodes with children. (It currently reads them as literally "grouping"; no information about the contents is provided). This change will be necessary for a later change I'm planning, wherein the role of parent nodes will be changed from "group" to "treeitem", in accordance with how roles are applied in WAI-ARIA reference examples such as https://www.w3.org/TR/wai-aria-practices/examples/treeview/treeview-1/treeview-1b.html * change aria-level binding to one-based * change role to treeitem * always set role to treeitem * simplify logic for setting role * add follow up TODO to makr role as deprecated Co-authored-by: Annie Wang --- src/cdk/tree/tree.spec.ts | 9 ++++++++ src/cdk/tree/tree.ts | 33 +++++++++------------------- src/material/tree/node.ts | 2 +- src/material/tree/tree.spec.ts | 11 ++++++++++ tools/public_api_guard/cdk/tree.d.ts | 1 - 5 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/cdk/tree/tree.spec.ts b/src/cdk/tree/tree.spec.ts index cff2df597175..784e64775394 100644 --- a/src/cdk/tree/tree.spec.ts +++ b/src/cdk/tree/tree.spec.ts @@ -120,6 +120,15 @@ describe('CdkTree', () => { })).toBe(true); }); + it('with the right aria-levels', () => { + // add a child to the first node + let data = dataSource.data; + dataSource.addChild(data[0], true); + + const ariaLevels = getNodes(treeElement).map(n => n.getAttribute('aria-level')); + expect(ariaLevels).toEqual(['2', '3', '2', '2']); + }); + it('with the right data', () => { expect(dataSource.data.length).toBe(3); diff --git a/src/cdk/tree/tree.ts b/src/cdk/tree/tree.ts index 585e3734cf5e..9f6778c1c103 100644 --- a/src/cdk/tree/tree.ts +++ b/src/cdk/tree/tree.ts @@ -22,18 +22,18 @@ import { OnDestroy, OnInit, QueryList, + TrackByFunction, ViewChild, ViewContainerRef, - ViewEncapsulation, - TrackByFunction + ViewEncapsulation } from '@angular/core'; import { BehaviorSubject, + isObservable, Observable, of as observableOf, Subject, Subscription, - isObservable, } from 'rxjs'; import {takeUntil} from 'rxjs/operators'; import {TreeControl} from './control/tree-control'; @@ -299,7 +299,7 @@ export class CdkTree implements AfterContentChecked, CollectionViewer, OnDest exportAs: 'cdkTreeNode', host: { '[attr.aria-expanded]': 'isExpanded', - '[attr.aria-level]': 'role === "treeitem" ? level : null', + '[attr.aria-level]': 'level + 1', '[attr.role]': 'role', 'class': 'cdk-tree-node', }, @@ -337,9 +337,9 @@ export class CdkTreeNode implements FocusableOption, OnDestroy { } /** - * The role of the node should be 'group' if it's an internal node, - * and 'treeitem' if it's a leaf node. + * The role of the node should always be 'treeitem'. */ + // TODO: mark as deprecated @Input() role: 'treeitem' | 'group' = 'treeitem'; constructor(protected _elementRef: ElementRef, @@ -364,24 +364,11 @@ export class CdkTreeNode implements FocusableOption, OnDestroy { this._elementRef.nativeElement.focus(); } + // TODO: role should eventually just be set in the component host protected _setRoleFromData(): void { - if (this._tree.treeControl.isExpandable) { - this.role = this._tree.treeControl.isExpandable(this._data) ? 'group' : 'treeitem'; - } else { - if (!this._tree.treeControl.getChildren) { - throw getTreeControlFunctionsMissingError(); - } - const childrenNodes = this._tree.treeControl.getChildren(this._data); - if (Array.isArray(childrenNodes)) { - this._setRoleFromChildren(childrenNodes as T[]); - } else if (isObservable(childrenNodes)) { - childrenNodes.pipe(takeUntil(this._destroyed)) - .subscribe(children => this._setRoleFromChildren(children)); - } + if (!this._tree.treeControl.isExpandable && !this._tree.treeControl.getChildren) { + throw getTreeControlFunctionsMissingError(); } - } - - protected _setRoleFromChildren(children: T[]) { - this.role = children && children.length ? 'group' : 'treeitem'; + this.role = 'treeitem'; } } diff --git a/src/material/tree/node.ts b/src/material/tree/node.ts index 06e689dff7d7..c7fd9b128477 100644 --- a/src/material/tree/node.ts +++ b/src/material/tree/node.ts @@ -44,7 +44,7 @@ const _MatTreeNodeMixinBase: HasTabIndexCtor & CanDisableCtor & typeof CdkTreeNo inputs: ['disabled', 'tabIndex'], host: { '[attr.aria-expanded]': 'isExpanded', - '[attr.aria-level]': 'role === "treeitem" ? level : null', + '[attr.aria-level]': 'level + 1', '[attr.role]': 'role', 'class': 'mat-tree-node' }, diff --git a/src/material/tree/tree.spec.ts b/src/material/tree/tree.spec.ts index c9b3ad2d9e36..9934b345e8d9 100644 --- a/src/material/tree/tree.spec.ts +++ b/src/material/tree/tree.spec.ts @@ -64,6 +64,17 @@ describe('MatTree', () => { }); }); + it('with the right aria-level attrs', () => { + // add a child to the first node + let data = underlyingDataSource.data; + underlyingDataSource.addChild(data[2]); + component.treeControl.expandAll(); + fixture.detectChanges(); + + const ariaLevels = getNodes(treeElement).map(n => n.getAttribute('aria-level')); + expect(ariaLevels).toEqual(['1', '1', '1', '2']); + }); + it('with the right data', () => { expect(underlyingDataSource.data.length).toBe(3); diff --git a/tools/public_api_guard/cdk/tree.d.ts b/tools/public_api_guard/cdk/tree.d.ts index 108112677ac3..6c2cfc6082d9 100644 --- a/tools/public_api_guard/cdk/tree.d.ts +++ b/tools/public_api_guard/cdk/tree.d.ts @@ -74,7 +74,6 @@ export declare class CdkTreeNode implements FocusableOption, OnDestroy { get level(): number; role: 'treeitem' | 'group'; constructor(_elementRef: ElementRef, _tree: CdkTree); - protected _setRoleFromChildren(children: T[]): void; protected _setRoleFromData(): void; focus(): void; ngOnDestroy(): void;