refactor(cdk-experimental/menu): add more robust implementation of getLabel for menu item (#19979)

When determining the text to use for a menu item for TypeAhead ensure that text in nested elements
isn't skipped and icons are skipped.

Co-authored-by: Niels Rasmussen <[email protected]>

Co-authored-by: Niels Rasmussen <[email protected]>

Author: andy9775

src/cdk-experimental/menu/menu-item.spec.ts

@@ -1,4 +1,4 @@
-import {Component} from '@angular/core';
+import {Component, Type} from '@angular/core';
 import {ComponentFixture, TestBed, async} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {CdkMenuModule} from './menu-module';
@@ -61,27 +61,59 @@ describe('MenuItem', () => {
   });
 
   describe('with complex inner elements', () => {
-    let fixture: ComponentFixture<ComplexMenuItem>;
     let menuItem: CdkMenuItem;
 
-    beforeEach(async(() => {
+    /**
+     * Build a component for testing and render it.
+     * @param componentClass the component to create
+     */
+    function createComponent<T>(componentClass: Type<T>) {
+      let fixture: ComponentFixture<T>;
+
       TestBed.configureTestingModule({
         imports: [CdkMenuModule],
-        declarations: [ComplexMenuItem],
+        declarations: [componentClass, MatIcon],
         providers: [{provide: CDK_MENU, useClass: CdkMenu}],
       }).compileComponents();
-    }));
 
-    beforeEach(() => {
-      fixture = TestBed.createComponent(ComplexMenuItem);
+      fixture = TestBed.createComponent(componentClass);
       fixture.detectChanges();
 
       menuItem = fixture.debugElement.query(By.directive(CdkMenuItem)).injector.get(CdkMenuItem);
+    }
+
+    it('should get the text for a simple menu item with no nested or wrapped elements', () => {
+      createComponent(SingleMenuItem);
+      expect(menuItem.getLabel()).toEqual('Click me!');
     });
 
-    it('should be able to extract the label text, with text nested in bold tag', () => {
-      expect(menuItem.getLabel()).toBe('Click me!');
+    it('should get the text for menu item with a single nested mat icon component', () => {
+      createComponent(MenuItemWithIcon);
+      expect(menuItem.getLabel()).toEqual('Click me!');
     });
+
+    it(
+      'should get the text for menu item with single nested component with the material ' +
+        'icon class',
+      () => {
+        createComponent(MenuItemWithIconClass);
+        expect(menuItem.getLabel()).toEqual('Click me!');
+      }
+    );
+
+    it('should get the text for a menu item with bold marked text', () => {
+      createComponent(MenuItemWithBoldElement);
+      expect(menuItem.getLabel()).toEqual('Click me!');
+    });
+
+    it(
+      'should get the text for a menu item with nested icon, nested icon class and nested ' +
+        'wrapping elements',
+      () => {
+        createComponent(MenuItemWithMultipleNestings);
+        expect(menuItem.getLabel()).toEqual('Click me!');
+      }
+    );
   });
 });
 
@@ -91,6 +123,47 @@ describe('MenuItem', () => {
 class SingleMenuItem {}
 
 @Component({
-  template: ` <button cdkMenuItem>Click <b>me!</b></button> `,
+  template: `
+    <button cdkMenuItem>
+      <mat-icon>unicorn</mat-icon>
+      Click me!
+    </button>
+  `,
+})
+class MenuItemWithIcon {}
+@Component({
+  template: `
+    <button cdkMenuItem>
+      <div class="material-icons">unicorn</div>
+      Click me!
+    </button>
+  `,
+})
+class MenuItemWithIconClass {}
+
+@Component({
+  template: ` <button cdkMenuItem><b>Click</b> me!</button> `,
+})
+class MenuItemWithBoldElement {}
+
+@Component({
+  template: `
+    <button cdkMenuItem>
+      <div>
+        <div class="material-icons">unicorn</div>
+        <div>
+          Click
+        </div>
+        <mat-icon>menu</mat-icon>
+        <div>me<b>!</b></div>
+      </div>
+    </button>
+  `,
+})
+class MenuItemWithMultipleNestings {}
+
+@Component({
+  selector: 'mat-icon',
+  template: '<ng-content></ng-content>',
 })
-class ComplexMenuItem {}
+class MatIcon {}

src/cdk-experimental/menu/menu-item.ts

@@ -25,6 +25,14 @@ import {CdkMenuItemTrigger} from './menu-item-trigger';
 import {Menu, CDK_MENU} from './menu-interface';
 import {FocusNext} from './menu-stack';
 
+// TODO refactor this to be configurable allowing for custom elements to be removed
+/** Removes all icons from within the given element. */
+function removeIcons(element: Element) {
+  for (const icon of Array.from(element.querySelectorAll('mat-icon, .material-icons'))) {
+    icon.parentNode?.removeChild(icon);
+  }
+}
+
 /**
  * Directive which provides the ability for an element to be focused and navigated to using the
  * keyboard when residing in a CdkMenu, CdkMenuBar, or CdkMenuGroup. It performs user defined
@@ -106,8 +114,12 @@ export class CdkMenuItem implements FocusableOption {
 
   /** Get the label for this element which is required by the FocusableOption interface. */
   getLabel(): string {
-    // TODO(andy): implement a more robust algorithm for determining nested text
-    return this._elementRef.nativeElement.textContent || '';
+    // TODO cloning the tree may be expensive; implement a better method
+    // we know that the current node is an element type
+    const clone = this._elementRef.nativeElement.cloneNode(true) as Element;
+    removeIcons(clone);
+
+    return clone.textContent?.trim() || '';
   }
 
   // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order