Skip to content

Commit

Permalink
fix(material/tree): Apply aria-level to all nodes (#17818)
Browse files Browse the repository at this point in the history
* 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 <annieyw@google.com>
  • Loading branch information
martiansnoop and annieyw committed Aug 6, 2020
1 parent a8ab040 commit aeb6f89
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 25 deletions.
9 changes: 9 additions & 0 deletions src/cdk/tree/tree.spec.ts
Expand Up @@ -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);

Expand Down
33 changes: 10 additions & 23 deletions src/cdk/tree/tree.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -299,7 +299,7 @@ export class CdkTree<T> 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',
},
Expand Down Expand Up @@ -337,9 +337,9 @@ export class CdkTreeNode<T> 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<HTMLElement>,
Expand All @@ -364,24 +364,11 @@ export class CdkTreeNode<T> 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';
}
}
2 changes: 1 addition & 1 deletion src/material/tree/node.ts
Expand Up @@ -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'
},
Expand Down
11 changes: 11 additions & 0 deletions src/material/tree/tree.spec.ts
Expand Up @@ -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);

Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/cdk/tree.d.ts
Expand Up @@ -74,7 +74,6 @@ export declare class CdkTreeNode<T> implements FocusableOption, OnDestroy {
get level(): number;
role: 'treeitem' | 'group';
constructor(_elementRef: ElementRef<HTMLElement>, _tree: CdkTree<T>);
protected _setRoleFromChildren(children: T[]): void;
protected _setRoleFromData(): void;
focus(): void;
ngOnDestroy(): void;
Expand Down

0 comments on commit aeb6f89

Please sign in to comment.