fix(material/chips): refactor non-interactive actions to prevent adding click handlers (#31664)

Author: adolgachev

goldens/material/chips/index.api.md

@@ -524,7 +524,7 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
     protected _originatesFromChip(event: Event): boolean;
     get role(): string | null;
     set role(value: string | null);
-    protected _skipPredicate(action: MatChipAction): boolean;
+    protected _skipPredicate(action: MatChipContent): boolean;
     protected _syncChipsState(): void;
     tabIndex: number;
     // (undocumented)
@@ -555,8 +555,7 @@ export interface MatChipTextControl {
 }
 
 // @public
-export class MatChipTrailingIcon extends MatChipAction {
-    isInteractive: boolean;
+export class MatChipTrailingIcon extends MatChipContent {
     // (undocumented)
     _isPrimary: boolean;
     // (undocumented)

src/material/chips/chip-action.ts

@@ -20,25 +20,22 @@ import {_CdkPrivateStyleLoader} from '@angular/cdk/private';
 import {_StructuralStylesLoader} from '../core';
 
 /**
- * Section within a chip.
+ * A non-interactive section of a chip.
  * @docs-private
  */
 @Directive({
-  selector: '[matChipAction]',
+  selector: '[matChipContent]',
   host: {
-    'class': 'mdc-evolution-chip__action mat-mdc-chip-action',
+    'class':
+      'mat-mdc-chip-action mdc-evolution-chip__action mdc-evolution-chip__action--presentational',
     '[class.mdc-evolution-chip__action--primary]': '_isPrimary',
-    '[class.mdc-evolution-chip__action--presentational]': '!isInteractive',
     '[class.mdc-evolution-chip__action--secondary]': '!_isPrimary',
     '[class.mdc-evolution-chip__action--trailing]': '!_isPrimary && !_isLeading',
-    '[attr.tabindex]': '_getTabindex()',
     '[attr.disabled]': '_getDisabledAttribute()',
     '[attr.aria-disabled]': 'disabled',
-    '(click)': '_handleClick($event)',
-    '(keydown)': '_handleKeydown($event)',
   },
 })
-export class MatChipAction {
+export class MatChipContent {
   _elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
   protected _parentChip = inject<{
     _handlePrimaryActionInteraction(): void;
@@ -48,9 +45,6 @@ export class MatChipAction {
     _isEditing?: boolean;
   }>(MAT_CHIP);
 
-  /** Whether the action is interactive. */
-  @Input() isInteractive = true;
-
   /** Whether this is the primary action in the chip. */
   _isPrimary = true;
 
@@ -88,15 +82,6 @@ export class MatChipAction {
     return this.disabled && !this._allowFocusWhenDisabled ? '' : null;
   }
 
-  /**
-   * Determine the value of the tabindex attribute for this chip action.
-   */
-  protected _getTabindex(): string | null {
-    return (this.disabled && !this._allowFocusWhenDisabled) || !this.isInteractive
-      ? null
-      : this.tabIndex.toString();
-  }
-
   constructor(...args: unknown[]);
 
   constructor() {
@@ -109,9 +94,31 @@ export class MatChipAction {
   focus() {
     this._elementRef.nativeElement.focus();
   }
+}
+
+/**
+ * Interactive section of a chip.
+ * @docs-private
+ */
+@Directive({
+  selector: '[matChipAction]',
+  host: {
+    '[attr.tabindex]': '_getTabindex()',
+    '[class.mdc-evolution-chip__action--presentational]': 'false',
+    '(click)': '_handleClick($event)',
+    '(keydown)': '_handleKeydown($event)',
+  },
+})
+export class MatChipAction extends MatChipContent {
+  /**
+   * Determine the value of the tabindex attribute for this chip action.
+   */
+  protected _getTabindex(): string | null {
+    return this.disabled && !this._allowFocusWhenDisabled ? null : this.tabIndex.toString();
+  }
 
   _handleClick(event: MouseEvent) {
-    if (!this.disabled && this.isInteractive && this._isPrimary) {
+    if (!this.disabled && this._isPrimary) {
       event.preventDefault();
       this._parentChip._handlePrimaryActionInteraction();
     }
@@ -121,7 +128,6 @@ export class MatChipAction {
     if (
       (event.keyCode === ENTER || event.keyCode === SPACE) &&
       !this.disabled &&
-      this.isInteractive &&
       this._isPrimary &&
       !this._parentChip._isEditing
     ) {

src/material/chips/chip-icons.ts

@@ -8,7 +8,7 @@
 
 import {ENTER, SPACE} from '@angular/cdk/keycodes';
 import {Directive} from '@angular/core';
-import {MatChipAction} from './chip-action';
+import {MatChipAction, MatChipContent} from './chip-action';
 import {MAT_CHIP_AVATAR, MAT_CHIP_EDIT, MAT_CHIP_REMOVE, MAT_CHIP_TRAILING_ICON} from './tokens';
 
 /** Avatar image within a chip. */
@@ -32,13 +32,7 @@ export class MatChipAvatar {}
   },
   providers: [{provide: MAT_CHIP_TRAILING_ICON, useExisting: MatChipTrailingIcon}],
 })
-export class MatChipTrailingIcon extends MatChipAction {
-  /**
-   * MDC considers all trailing actions as a remove icon,
-   * but we support non-interactive trailing icons.
-   */
-  override isInteractive = false;
-
+export class MatChipTrailingIcon extends MatChipContent {
   override _isPrimary = false;
 }
 

src/material/chips/chip-listbox.ts

@@ -398,6 +398,6 @@ export class MatChipListbox
     // https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/):
     //   "For the following composite widget elements, keep them focusable when disabled: Options in a
     //   Listbox..."
-    return !action.isInteractive;
+    return false;
   }
 }

src/material/chips/chip-row.spec.ts

@@ -458,19 +458,6 @@ describe('Row Chips', () => {
         fixture.detectChanges();
         expect(chipInstance._hasInteractiveActions()).toBe(true);
       });
-
-      it('should return false if all actions are non-interactive', () => {
-        // Make primary action non-interactive for testing purposes.
-        chipInstance.primaryAction.isInteractive = false;
-        testComponent.showTrailingIcon = true;
-        testComponent.removable = false; // remove icon is interactive
-        fixture.changeDetectorRef.markForCheck();
-        fixture.detectChanges();
-
-        // The trailing icon is not interactive.
-        expect(chipInstance.trailingIcon.isInteractive).toBe(false);
-        expect(chipInstance._hasInteractiveActions()).toBe(false);
-      });
     });
 
     describe('with edit icon', () => {

src/material/chips/chip-row.ts

@@ -61,7 +61,7 @@ export interface MatChipEditedEvent extends MatChipEvent {
     '[attr.aria-description]': 'null',
     '[attr.role]': 'role',
     '(focus)': '_handleFocus()',
-    '(click)': '_handleClick($event)',
+    '(click)': 'this._hasInteractiveActions() ? _handleClick($event) : null',
     '(dblclick)': '_handleDoubleclick($event)',
   },
   providers: [

src/material/chips/chip-set.ts

@@ -26,7 +26,7 @@ import {
 import {Observable, Subject, merge} from 'rxjs';
 import {startWith, switchMap, takeUntil} from 'rxjs/operators';
 import {MatChip, MatChipEvent} from './chip';
-import {MatChipAction} from './chip-action';
+import {MatChipAction, MatChipContent} from './chip-action';
 
 /**
  * Basic container component for the MatChip component.
@@ -266,10 +266,9 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
    * Determines if key manager should avoid putting a given chip action in the tab index. Skip
    * non-interactive and disabled actions since the user can't do anything with them.
    */
-  protected _skipPredicate(action: MatChipAction): boolean {
-    // Skip chips that the user cannot interact with. `mat-chip-set` does not permit focusing disabled
-    // chips.
-    return !action.isInteractive || action.disabled;
+  protected _skipPredicate(action: MatChipContent): boolean {
+    // `mat-chip-set` does not permit focusing disabled chips.
+    return action.disabled;
   }
 
   /** Listens to changes in the chip set and syncs up the state of the individual chips. */

src/material/chips/chip.html

@@ -1,7 +1,7 @@
 <span class="mat-mdc-chip-focus-overlay"></span>
 
 <span class="mdc-evolution-chip__cell mdc-evolution-chip__cell--primary">
-  <span matChipAction [isInteractive]="false">
+  <span matChipContent>
     @if (leadingIcon) {
       <span class="mdc-evolution-chip__graphic mat-mdc-chip-graphic">
         <ng-content select="mat-chip-avatar, [matChipAvatar]"></ng-content>

src/material/chips/chip.spec.ts

@@ -54,6 +54,17 @@ describe('MatChip', () => {
 
       expect(chip.getAttribute('tabindex')).toBe('15');
     });
+
+    it('should disable the ripple if there are no interactive actions', () => {
+      fixture = TestBed.createComponent(BasicChip);
+      fixture.detectChanges();
+
+      chipDebugElement = fixture.debugElement.query(By.directive(MatChip))!;
+      chipInstance = chipDebugElement.injector.get<MatChip>(MatChip);
+
+      expect(chipInstance._hasInteractiveActions()).toBe(false);
+      expect(chipInstance._isRippleDisabled()).toBe(true);
+    });
   });
 
   describe('MatChip', () => {
@@ -117,18 +128,6 @@ describe('MatChip', () => {
       expect(primaryAction.hasAttribute('tabindex')).toBe(false);
     });
 
-    it('should disable the ripple if there are no interactive actions', () => {
-      // expect(chipInstance._isRippleDisabled()).toBe(false); TODO(andreyd)
-
-      // Make primary action non-interactive for testing purposes.
-      chipInstance.primaryAction.isInteractive = false;
-      fixture.changeDetectorRef.markForCheck();
-      fixture.detectChanges();
-
-      expect(chipInstance._hasInteractiveActions()).toBe(false);
-      expect(chipInstance._isRippleDisabled()).toBe(true);
-    });
-
     it('should return the chip text if value is undefined', () => {
       expect(chipInstance.value.trim()).toBe(fixture.componentInstance.name);
     });

src/material/chips/chip.ts

@@ -43,7 +43,7 @@ import {
   _animationsDisabled,
 } from '../core';
 import {Subject, Subscription, merge} from 'rxjs';
-import {MatChipAction} from './chip-action';
+import {MatChipAction, MatChipContent} from './chip-action';
 import {MatChipAvatar, MatChipEdit, MatChipRemove, MatChipTrailingIcon} from './chip-icons';
 import {
   MAT_CHIP,
@@ -93,7 +93,7 @@ export interface MatChipEvent {
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
   providers: [{provide: MAT_CHIP, useExisting: MatChip}],
-  imports: [MatChipAction],
+  imports: [MatChipContent],
 })
 export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck, OnDestroy {
   _changeDetectorRef = inject(ChangeDetectorRef);
@@ -386,10 +386,6 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
       result.push(this.removeIcon);
     }
 
-    if (this.trailingIcon) {
-      result.push(this.trailingIcon);
-    }
-
     return result;
   }
 
@@ -400,7 +396,7 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
 
   /** Returns whether the chip has any interactive actions. */
   _hasInteractiveActions(): boolean {
-    return this._getActions().some(a => a.isInteractive);
+    return this._getActions().length > 0;
   }
 
   /** Handles interactions with the edit action of the chip. */