fix(material/chips): simplify repeat chip removal prevention (#29048)

We have some logic that prevents chip removal if the users is holding down the backspace key. It currently breaks down in the case where a value is pasted into the input via click.

These changes simplify our detection by using `KeyboardEvent.repeat` which also resolves the paste issue.

Author: crisbeto

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

@@ -1,7 +1,6 @@
 import {animate, style, transition, trigger} from '@angular/animations';
 import {Direction, Directionality} from '@angular/cdk/bidi';
 import {
-  A,
   BACKSPACE,
   DELETE,
   END,
@@ -13,6 +12,8 @@ import {
   TAB,
 } from '@angular/cdk/keycodes';
 import {
+  createKeyboardEvent,
+  dispatchEvent,
   dispatchFakeEvent,
   dispatchKeyboardEvent,
   patchElementFocus,
@@ -789,28 +790,15 @@ describe('MDC-based MatChipGrid', () => {
         expectLastCellFocused();
       });
 
-      it(
-        'should not focus the last chip when pressing BACKSPACE after changing input, ' +
-          'until BACKSPACE is released and pressed again',
-        () => {
-          // Change the input
-          dispatchKeyboardEvent(nativeInput, 'keydown', A);
-
-          // It shouldn't focus until backspace is released and pressed again
-          dispatchKeyboardEvent(nativeInput, 'keydown', BACKSPACE);
-          dispatchKeyboardEvent(nativeInput, 'keydown', BACKSPACE);
-          dispatchKeyboardEvent(nativeInput, 'keydown', BACKSPACE);
-          expectNoCellFocused();
-
-          // Still not focused
-          dispatchKeyboardEvent(nativeInput, 'keyup', BACKSPACE);
-          expectNoCellFocused();
-
-          // Only now should it focus the last element
-          dispatchKeyboardEvent(nativeInput, 'keydown', BACKSPACE);
-          expectLastCellFocused();
-        },
-      );
+      it('should not focus the last chip when the BACKSPACE key is being repeated', () => {
+        // Only now should it focus the last element
+        const event = createKeyboardEvent('keydown', BACKSPACE);
+        Object.defineProperty(event, 'repeat', {
+          get: () => true,
+        });
+        dispatchEvent(nativeInput, event);
+        expectNoCellFocused();
+      });
 
       it('should focus last chip after pressing BACKSPACE after creating a chip', () => {
         // Create a chip

src/material/chips/chip-input.ts

@@ -8,7 +8,6 @@
 
 import {BACKSPACE, hasModifierKey} from '@angular/cdk/keycodes';
 import {
-  AfterContentInit,
   Directive,
   ElementRef,
   EventEmitter,
@@ -57,7 +56,6 @@ let nextUniqueId = 0;
     // the MDC chips were landed initially with it.
     'class': 'mat-mdc-chip-input mat-mdc-input-element mdc-text-field__input mat-input-element',
     '(keydown)': '_keydown($event)',
-    '(keyup)': '_keyup($event)',
     '(blur)': '_blur()',
     '(focus)': '_focus()',
     '(input)': '_onInput()',
@@ -70,10 +68,7 @@ let nextUniqueId = 0;
   },
   standalone: true,
 })
-export class MatChipInput implements MatChipTextControl, AfterContentInit, OnChanges, OnDestroy {
-  /** Used to prevent focus moving to chips while user is holding backspace */
-  private _focusLastChipOnBackspace: boolean;
-
+export class MatChipInput implements MatChipTextControl, OnChanges, OnDestroy {
   /** Whether the control is focused. */
   focused: boolean = false;
 
@@ -153,36 +148,17 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
     this.chipEnd.complete();
   }
 
-  ngAfterContentInit(): void {
-    this._focusLastChipOnBackspace = this.empty;
-  }
-
   /** Utility method to make host definition/tests more clear. */
-  _keydown(event?: KeyboardEvent) {
-    if (event) {
-      // To prevent the user from accidentally deleting chips when pressing BACKSPACE continuously,
-      // We focus the last chip on backspace only after the user has released the backspace button,
-      // And the input is empty (see behaviour in _keyup)
-      if (event.keyCode === BACKSPACE && this._focusLastChipOnBackspace) {
+  _keydown(event: KeyboardEvent) {
+    if (this.empty && event.keyCode === BACKSPACE) {
+      // Ignore events where the user is holding down backspace
+      // so that we don't accidentally remove too many chips.
+      if (!event.repeat) {
         this._chipGrid._focusLastChip();
-        event.preventDefault();
-        return;
-      } else {
-        this._focusLastChipOnBackspace = false;
       }
-    }
-
-    this._emitChipEnd(event);
-  }
-
-  /**
-   * Pass events to the keyboard manager. Available here for tests.
-   */
-  _keyup(event: KeyboardEvent) {
-    // Allow user to move focus to chips next time he presses backspace
-    if (!this._focusLastChipOnBackspace && event.keyCode === BACKSPACE && this.empty) {
-      this._focusLastChipOnBackspace = true;
       event.preventDefault();
+    } else {
+      this._emitChipEnd(event);
     }
   }
 
@@ -201,7 +177,6 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
 
   _focus() {
     this.focused = true;
-    this._focusLastChipOnBackspace = this.empty;
     this._chipGrid.stateChanges.next();
   }
 
@@ -231,7 +206,6 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
   /** Clears the input */
   clear(): void {
     this.inputElement.value = '';
-    this._focusLastChipOnBackspace = true;
   }
 
   setDescribedByIds(ids: string[]): void {

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

@@ -123,7 +123,7 @@ describe('MDC-based Row Chips', () => {
 
           spyOn(testComponent, 'chipRemove');
 
-          chipInstance._handleKeydown(DELETE_EVENT);
+          dispatchEvent(chipNativeElement, DELETE_EVENT);
           fixture.detectChanges();
 
           expect(testComponent.chipRemove).toHaveBeenCalled();
@@ -134,11 +134,25 @@ describe('MDC-based Row Chips', () => {
 
           spyOn(testComponent, 'chipRemove');
 
-          chipInstance._handleKeydown(BACKSPACE_EVENT);
+          dispatchEvent(chipNativeElement, BACKSPACE_EVENT);
           fixture.detectChanges();
 
           expect(testComponent.chipRemove).toHaveBeenCalled();
         });
+
+        it('should not remove for repeated BACKSPACE event', () => {
+          const BACKSPACE_EVENT = createKeyboardEvent('keydown', BACKSPACE);
+          Object.defineProperty(BACKSPACE_EVENT, 'repeat', {
+            get: () => true,
+          });
+
+          spyOn(testComponent, 'chipRemove');
+
+          dispatchEvent(chipNativeElement, BACKSPACE_EVENT);
+          fixture.detectChanges();
+
+          expect(testComponent.chipRemove).not.toHaveBeenCalled();
+        });
       });
 
       describe('when removable is false', () => {
@@ -152,7 +166,7 @@ describe('MDC-based Row Chips', () => {
 
           spyOn(testComponent, 'chipRemove');
 
-          chipInstance._handleKeydown(DELETE_EVENT);
+          dispatchEvent(chipNativeElement, DELETE_EVENT);
           fixture.detectChanges();
 
           expect(testComponent.chipRemove).not.toHaveBeenCalled();
@@ -164,7 +178,7 @@ describe('MDC-based Row Chips', () => {
           spyOn(testComponent, 'chipRemove');
 
           // Use the delete to remove the chip
-          chipInstance._handleKeydown(BACKSPACE_EVENT);
+          dispatchEvent(chipNativeElement, BACKSPACE_EVENT);
           fixture.detectChanges();
 
           expect(testComponent.chipRemove).not.toHaveBeenCalled();

src/material/chips/chip.ts

@@ -336,7 +336,9 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
 
   /** Handles keyboard events on the chip. */
   _handleKeydown(event: KeyboardEvent) {
-    if (event.keyCode === BACKSPACE || event.keyCode === DELETE) {
+    // Ignore backspace events where the user is holding down the key
+    // so that we don't accidentally remove too many chips.
+    if ((event.keyCode === BACKSPACE && !event.repeat) || event.keyCode === DELETE) {
       event.preventDefault();
       this.remove();
     }

tools/public_api_guard/material/chips.md

@@ -249,7 +249,7 @@ export class MatChipGridChange {
 }
 
 // @public
-export class MatChipInput implements MatChipTextControl, AfterContentInit, OnChanges, OnDestroy {
+export class MatChipInput implements MatChipTextControl, OnChanges, OnDestroy {
     constructor(_elementRef: ElementRef<HTMLInputElement>, defaultOptions: MatChipsDefaultOptions, formField?: MatFormField);
     addOnBlur: boolean;
     _blur(): void;
@@ -269,15 +269,12 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
     focused: boolean;
     id: string;
     readonly inputElement: HTMLInputElement;
-    _keydown(event?: KeyboardEvent): void;
-    _keyup(event: KeyboardEvent): void;
+    _keydown(event: KeyboardEvent): void;
     // (undocumented)
     static ngAcceptInputType_addOnBlur: unknown;
     // (undocumented)
     static ngAcceptInputType_disabled: unknown;
     // (undocumented)
-    ngAfterContentInit(): void;
-    // (undocumented)
     ngOnChanges(): void;
     // (undocumented)
     ngOnDestroy(): void;