fix(cdk/tree): fix level for nested tree nodes (#20226)

* fix(cdk/tree): fix level for flat and nested tree nodes

* add test for material/tree

* cache parent level instead of traversing DOM on ever level get

* logging

* set aria-level in ngOnInit, clean up

* public api

* change host binding to @HostBinding

* use host:{} for mat tree and hostbinding for cdk tree

* fix tslint comments, fix class inheritance for mat tree

* clarify adding class directly to elementRef instead of on host property

* minor fixes

* fix isNodeElement method name/logic, guarantee boolean value

* clean up duplicate @HostBinding and host:{}

* replace isDevMode()

* super life cycle hooks

* public api update

* comments explaining super life hook calls, add aria-expanded with setAttribute

* set aria-expaded in ngDoCheck

* remove duplicate input from MatTreeNode, add missing inputs:[]  for classes inheriting CdkTreeNode

* add super.ngDoCheck()

* change isExpanded setter to private method

* remove setting aria-expanded in constructor

Author: annieyw

src/cdk/tree/nested-node.ts

@@ -9,10 +9,12 @@ import {
   AfterContentInit,
   ContentChildren,
   Directive,
+  DoCheck,
   ElementRef,
   IterableDiffer,
   IterableDiffers,
   OnDestroy,
+  OnInit,
   QueryList,
 } from '@angular/core';
 import {isObservable} from 'rxjs';
@@ -31,17 +33,15 @@ import {getTreeControlFunctionsMissingError} from './tree-errors';
 @Directive({
   selector: 'cdk-nested-tree-node',
   exportAs: 'cdkNestedTreeNode',
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.role]': 'role',
-    'class': 'cdk-tree-node cdk-nested-tree-node',
-  },
+  inputs: ['role', 'disabled', 'tabIndex'],
   providers: [
     {provide: CdkTreeNode, useExisting: CdkNestedTreeNode},
     {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: CdkNestedTreeNode}
   ]
 })
-export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContentInit, OnDestroy {
+export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContentInit, DoCheck,
+  OnDestroy,
+  OnInit {
   /** Differ used to find the changes in the data provided by the data source. */
   private _dataDiffer: IterableDiffer<T>;
 
@@ -60,6 +60,11 @@ export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContent
               protected _tree: CdkTree<T>,
               protected _differs: IterableDiffers) {
     super(_elementRef, _tree);
+    // The classes are directly added here instead of in the host property because classes on
+    // the host property are not inherited with View Engine. It is not set as a @HostBinding because
+    // it is not set by the time it's children nodes try to read the class from it.
+    // TODO: move to host after View Engine deprecation
+    this._elementRef.nativeElement.classList.add('cdk-nested-tree-node');
   }
 
   ngAfterContentInit() {
@@ -78,6 +83,16 @@ export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContent
         .subscribe(() => this.updateChildrenNodes());
   }
 
+  // This is a workaround for https://github.com/angular/angular/issues/23091
+  // In aot mode, the lifecycle hooks from parent class are not called.
+  ngOnInit() {
+    super.ngOnInit();
+  }
+
+  ngDoCheck() {
+    super.ngDoCheck();
+  }
+
   ngOnDestroy() {
     this._clear();
     super.ngOnDestroy();

src/cdk/tree/tree.spec.ts

@@ -129,6 +129,23 @@ describe('CdkTree', () => {
         expect(ariaLevels).toEqual(['2', '3', '2', '2']);
       });
 
+      it('with the right aria-expanded attrs', () => {
+        // add a child to the first node
+        let data = dataSource.data;
+        dataSource.addChild(data[2]);
+        fixture.detectChanges();
+        expect(getNodes(treeElement).every(node => {
+          return node.getAttribute('aria-expanded') === 'false';
+        })).toBe(true);
+
+        component.treeControl.expandAll();
+        fixture.detectChanges();
+
+        expect(getNodes(treeElement).every(node => {
+          return node.getAttribute('aria-expanded') === 'true';
+        })).toBe(true);
+      });
+
       it('with the right data', () => {
         expect(dataSource.data.length).toBe(3);
 
@@ -595,6 +612,21 @@ describe('CdkTree', () => {
             [_, _, `topping_6 - cheese_6 + base_6`],
             [`topping_3 - cheese_3 + base_3`]);
       });
+
+      it('with correct aria-level on nodes', () => {
+        expect(getNodes(treeElement).every(node => {
+          return node.getAttribute('aria-level') === '1';
+        })).toBe(true);
+
+        let data = dataSource.data;
+        const child = dataSource.addChild(data[1], false);
+        dataSource.addChild(child, false);
+        fixture.detectChanges();
+
+        const nodes = getNodes(treeElement);
+        const levels = nodes.map(n => n.getAttribute('aria-level'));
+        expect(levels).toEqual(['1', '1', '2', '3', '1']);
+      });
     });
 
     describe('with static children', () => {
@@ -676,6 +708,24 @@ describe('CdkTree', () => {
         treeElement = fixture.nativeElement.querySelector('cdk-tree');
       });
 
+      it('with the right aria-expanded attrs', () => {
+        expect(getNodes(treeElement).every(node => {
+          return node.getAttribute('aria-expanded') === 'false';
+        })).toBe(true);
+
+        component.toggleRecursively = false;
+        let data = dataSource.data;
+        const child = dataSource.addChild(data[1], false);
+        dataSource.addChild(child, false);
+        fixture.detectChanges();
+
+        (getNodes(treeElement)[1] as HTMLElement).click();
+        fixture.detectChanges();
+
+        const ariaExpanded = getNodes(treeElement).map(n => n.getAttribute('aria-expanded'));
+        expect(ariaExpanded).toEqual(['false', 'true', 'false', 'false']);
+      });
+
       it('should expand/collapse the node multiple times', () => {
         component.toggleRecursively = false;
         let data = dataSource.data;

src/cdk/tree/tree.ts

@@ -14,6 +14,7 @@ import {
   Component,
   ContentChildren,
   Directive,
+  DoCheck,
   ElementRef,
   Input,
   IterableChangeRecord,
@@ -46,6 +47,7 @@ import {
   getTreeMultipleDefaultNodeDefsError,
   getTreeNoValidDataSourceError
 } from './tree-errors';
+import {coerceNumberProperty} from '@angular/cdk/coercion';
 
 /**
  * CDK tree component that connects with a data source to retrieve data of type `T` and renders
@@ -300,14 +302,21 @@ export class CdkTree<T> implements AfterContentChecked, CollectionViewer, OnDest
 @Directive({
   selector: 'cdk-tree-node',
   exportAs: 'cdkTreeNode',
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.aria-level]': 'level + 1',
-    '[attr.role]': 'role',
-    'class': 'cdk-tree-node',
-  },
 })
-export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
+export class CdkTreeNode<T> implements DoCheck, FocusableOption, OnDestroy, OnInit {
+  /**
+   * The role of the tree node.
+   * @deprecated The correct role is 'treeitem', 'group' should not be used. This input will be
+   *   removed in a future version.
+   * @breaking-change 12.0.0 Remove this input
+   */
+  @Input() get role(): 'treeitem'|'group' { return 'treeitem'; }
+
+  set role(_role: 'treeitem'|'group') {
+    // TODO: move to host after View Engine deprecation
+    this._elementRef.nativeElement.setAttribute('role', _role);
+  }
+
   /**
    * The most recently created `CdkTreeNode`. We save it in static variable so we can retrieve it
    * in `CdkTree` and set the data to it.
@@ -320,6 +329,8 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
   /** Emits when the node's data has changed. */
   _dataChanges = new Subject<void>();
 
+  private _parentNodeAriaLevel: number;
+
   /** The tree node's data. */
   get data(): T { return this._data; }
   set data(value: T) {
@@ -335,19 +346,45 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
     return this._tree.treeControl.isExpanded(this._data);
   }
 
-  get level(): number {
-    return this._tree.treeControl.getLevel ? this._tree.treeControl.getLevel(this._data) : 0;
+  private _setExpanded(_expanded: boolean) {
+    this._isAriaExpanded = _expanded;
+    this._elementRef.nativeElement.setAttribute('aria-expanded', `${_expanded}`);
   }
 
-  /**
-   * The role of the node should always be 'treeitem'.
-   */
-  // TODO: mark as deprecated
-  @Input() role: 'treeitem' | 'group' = 'treeitem';
+  protected _isAriaExpanded: boolean;
+
+  get level(): number {
+   // If the treeControl has a getLevel method, use it to get the level. Otherwise read the
+   // aria-level off the parent node and use it as the level for this node (note aria-level is
+   // 1-indexed, while this property is 0-indexed, so we don't need to increment).
+   return this._tree.treeControl.getLevel ?
+     this._tree.treeControl.getLevel(this._data) : this._parentNodeAriaLevel;
+   }
 
   constructor(protected _elementRef: ElementRef<HTMLElement>,
               protected _tree: CdkTree<T>) {
     CdkTreeNode.mostRecentTreeNode = this as CdkTreeNode<T>;
+    // The classes are directly added here instead of in the host property because classes on
+    // the host property are not inherited with View Engine. It is not set as a @HostBinding because
+    // it is not set by the time it's children nodes try to read the class from it.
+    // TODO: move to host after View Engine deprecation
+    this._elementRef.nativeElement.classList.add('cdk-tree-node');
+    this.role = 'treeitem';
+  }
+
+  ngOnInit(): void {
+    this._parentNodeAriaLevel = getParentNodeAriaLevel(this._elementRef.nativeElement);
+    this._elementRef.nativeElement.setAttribute('aria-level', `${this.level + 1}`);
+  }
+
+  ngDoCheck() {
+    // aria-expanded is be set here because the expanded state is stored in the tree control and
+    // the node isn't aware when the state is changed.
+    // It is not set using a @HostBinding because they sometimes get lost with Mixin based classes.
+    // TODO: move to host after View Engine deprecation
+    if (this.isExpanded != this._isAriaExpanded) {
+      this._setExpanded(this.isExpanded);
+    }
   }
 
   ngOnDestroy() {
@@ -376,3 +413,27 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
     this.role = 'treeitem';
   }
 }
+
+function getParentNodeAriaLevel(nodeElement: HTMLElement): number {
+  let parent = nodeElement.parentElement;
+  while (parent && !isNodeElement(parent)) {
+    parent = parent.parentElement;
+  }
+  if (!parent) {
+    if (typeof ngDevMode === 'undefined' || ngDevMode) {
+      throw Error('Incorrect tree structure containing detached node.');
+    } else {
+      return -1;
+    }
+  } else if (parent.classList.contains('cdk-nested-tree-node')) {
+    return coerceNumberProperty(parent.getAttribute('aria-level')!);
+  } else {
+    // The ancestor element is the cdk-tree itself
+    return 0;
+  }
+}
+
+function isNodeElement(element: HTMLElement) {
+  const classList = element.classList;
+  return !!(classList?.contains('cdk-nested-tree-node') || classList?.contains('cdk-tree'));
+}

src/material/tree/node.ts

@@ -17,10 +17,11 @@ import {
   AfterContentInit,
   Attribute,
   Directive,
+  DoCheck,
   ElementRef,
   Input,
   IterableDiffers,
-  OnDestroy,
+  OnDestroy, OnInit,
 } from '@angular/core';
 import {
   CanDisable,
@@ -41,25 +42,38 @@ const _MatTreeNodeMixinBase: HasTabIndexCtor & CanDisableCtor & typeof CdkTreeNo
 @Directive({
   selector: 'mat-tree-node',
   exportAs: 'matTreeNode',
-  inputs: ['disabled', 'tabIndex'],
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.aria-level]': 'level + 1',
-    '[attr.role]': 'role',
-    'class': 'mat-tree-node'
-  },
+  inputs: ['role', 'disabled', 'tabIndex'],
   providers: [{provide: CdkTreeNode, useExisting: MatTreeNode}]
 })
 export class MatTreeNode<T> extends _MatTreeNodeMixinBase<T>
-    implements CanDisable, HasTabIndex {
-  @Input() role: 'treeitem' | 'group' = 'treeitem';
+    implements CanDisable, DoCheck, HasTabIndex, OnInit, OnDestroy {
+
 
   constructor(protected _elementRef: ElementRef<HTMLElement>,
               protected _tree: CdkTree<T>,
               @Attribute('tabindex') tabIndex: string) {
     super(_elementRef, _tree);
 
     this.tabIndex = Number(tabIndex) || 0;
+    // The classes are directly added here instead of in the host property because classes on
+    // the host property are not inherited with View Engine. It is not set as a @HostBinding because
+    // it is not set by the time it's children nodes try to read the class from it.
+    // TODO: move to host after View Engine deprecation
+    this._elementRef.nativeElement.classList.add('mat-tree-node');
+  }
+
+  // This is a workaround for https://github.com/angular/angular/issues/23091
+  // In aot mode, the lifecycle hooks from parent class are not called.
+  ngOnInit() {
+    super.ngOnInit();
+  }
+
+  ngDoCheck() {
+    super.ngDoCheck();
+  }
+
+  ngOnDestroy() {
+    super.ngOnDestroy();
   }
 
   static ngAcceptInputType_disabled: BooleanInput;
@@ -86,19 +100,15 @@ export class MatTreeNodeDef<T> extends CdkTreeNodeDef<T> {
 @Directive({
   selector: 'mat-nested-tree-node',
   exportAs: 'matNestedTreeNode',
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.role]': 'role',
-    'class': 'mat-nested-tree-node',
-  },
+  inputs: ['role', 'disabled', 'tabIndex'],
   providers: [
     {provide: CdkNestedTreeNode, useExisting: MatNestedTreeNode},
     {provide: CdkTreeNode, useExisting: MatNestedTreeNode},
     {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: MatNestedTreeNode}
   ]
 })
-export class MatNestedTreeNode<T> extends CdkNestedTreeNode<T> implements AfterContentInit,
-  OnDestroy {
+export class MatNestedTreeNode<T> extends CdkNestedTreeNode<T> implements AfterContentInit, DoCheck,
+  OnDestroy, OnInit {
   @Input('matNestedTreeNode') node: T;
 
   /** Whether the node is disabled. */
@@ -122,11 +132,24 @@ export class MatNestedTreeNode<T> extends CdkNestedTreeNode<T> implements AfterC
               @Attribute('tabindex') tabIndex: string) {
     super(_elementRef, _tree, _differs);
     this.tabIndex = Number(tabIndex) || 0;
+    // The classes are directly added here instead of in the host property because classes on
+    // the host property are not inherited with View Engine. It is not set as a @HostBinding because
+    // it is not set by the time it's children nodes try to read the class from it.
+    // TODO: move to host after View Engine deprecation
+    this._elementRef.nativeElement.classList.add('mat-nested-tree-node');
   }
 
   // This is a workaround for https://github.com/angular/angular/issues/23091
   // In aot mode, the lifecycle hooks from parent class are not called.
   // TODO(tinayuangao): Remove when the angular issue #23091 is fixed
+  ngOnInit() {
+    super.ngOnInit();
+  }
+
+  ngDoCheck() {
+    super.ngDoCheck();
+  }
+
   ngAfterContentInit() {
     super.ngAfterContentInit();
   }