fix(material-experimental/mdc-chips): decouple removal from animation (#21586)

Currently the MDC chips automatically trigger an animation that collapses the chip to 0x0
dimensions, but it still keeps it in the DOM unless the consumer updates their data source.
This is misleading and there's no way for the consumer to see it.

These changes bring the chip in line with our current behavior where we only dispatch the
remove event and then it's up to the consumer to remove the chip.

Making these changes also revealed some tests that were passing by accident and weren't
actually testing the things they were supposed to, because the `animationend` event wasn't
being dispatched. I've tweaked them in order to get them to work as expected.

Fixes #21561.

Author: crisbeto

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

@@ -12,7 +12,6 @@ import {
   TAB
 } from '@angular/cdk/keycodes';
 import {
-  createFakeEvent,
   createKeyboardEvent,
   dispatchEvent,
   dispatchFakeEvent,
@@ -216,7 +215,7 @@ describe('MDC-based MatChipGrid', () => {
           expect(chipGridInstance.focus).toHaveBeenCalled();
         });
 
-        it('should move focus to the last chip when the focused chip was deleted inside a' +
+        it('should move focus to the last chip when the focused chip was deleted inside a ' +
           'component with animations', fakeAsync(() => {
             fixture.destroy();
             TestBed.resetTestingModule();
@@ -602,15 +601,13 @@ describe('MDC-based MatChipGrid', () => {
 
   describe('with chip remove', () => {
     let chipGrid: MatChipGrid;
-    let chipElements: DebugElement[];
     let chipRemoveDebugElements: DebugElement[];
 
     beforeEach(() => {
       fixture = createComponent(ChipGridWithRemove);
       fixture.detectChanges();
 
       chipGrid = fixture.debugElement.query(By.directive(MatChipGrid))!.componentInstance;
-      chipElements = fixture.debugElement.queryAll(By.directive(MatChipRow));
       chipRemoveDebugElements = fixture.debugElement.queryAll(By.directive(MatChipRemove));
       chips = chipGrid._chips;
     });
@@ -623,12 +620,6 @@ describe('MDC-based MatChipGrid', () => {
       dispatchMouseEvent(chipRemoveDebugElements[2].nativeElement, 'click');
       fixture.detectChanges();
 
-      const fakeEvent = createFakeEvent('transitionend');
-      (fakeEvent as any).propertyName = 'width';
-      chipElements[2].nativeElement.dispatchEvent(fakeEvent);
-
-      fixture.detectChanges();
-
       expect(chips.toArray()[2].value).not.toBe(2, 'Expected the third chip to be removed.');
       expect(chipGrid._keyManager.activeRowIndex).toBe(2);
     });
@@ -771,9 +762,10 @@ describe('MDC-based MatChipGrid', () => {
 
       dispatchFakeEvent(nativeChips[0], 'focusout');
       fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
       zone.simulateZoneExit();
       fixture.detectChanges();
-      tick();
       expect(formField.classList).not.toContain('mat-focused');
     }));
 
@@ -787,10 +779,6 @@ describe('MDC-based MatChipGrid', () => {
         chip.focus();
         dispatchKeyboardEvent(chip, 'keydown', BACKSPACE);
         fixture.detectChanges();
-        const fakeEvent = createFakeEvent('transitionend');
-        (fakeEvent as any).propertyName = 'width';
-        chip.dispatchEvent(fakeEvent);
-        fixture.detectChanges();
         tick();
       });
 

src/material-experimental/mdc-chips/chip-grid.ts

@@ -505,7 +505,7 @@ export class MatChipGrid extends _MatChipGridMixinBase implements AfterContentIn
         const newChipIndex = Math.min(this._lastDestroyedChipIndex, this._chips.length - 1);
         this._keyManager.setActiveCell({
           row: newChipIndex,
-          column: this._keyManager.activeColumnIndex
+          column: Math.max(this._keyManager.activeColumnIndex, 0)
         });
       } else {
         this.focus();

src/material-experimental/mdc-chips/chip-remove.spec.ts

@@ -1,5 +1,4 @@
 import {
-  createFakeEvent,
   dispatchKeyboardEvent,
   createKeyboardEvent,
   dispatchEvent,
@@ -55,18 +54,6 @@ describe('MDC-based Chip Remove', () => {
       expect(buttonElement.hasAttribute('type')).toBe(false);
     });
 
-    it('should start MDC exit animation on click', () => {
-      let buttonElement = chipNativeElement.querySelector('button')!;
-
-      testChip.removable = true;
-      fixture.detectChanges();
-
-      buttonElement.click();
-      fixture.detectChanges();
-
-      expect(chipNativeElement.classList.contains('mdc-chip--exit')).toBe(true);
-    });
-
     it('should emit (removed) event when exit animation is complete', () => {
       let buttonElement = chipNativeElement.querySelector('button')!;
 
@@ -77,10 +64,6 @@ describe('MDC-based Chip Remove', () => {
       buttonElement.click();
       fixture.detectChanges();
 
-      const fakeEvent = createFakeEvent('transitionend');
-      (fakeEvent as any).propertyName = 'width';
-      chipNativeElement.dispatchEvent(fakeEvent);
-
       expect(testChip.didRemove).toHaveBeenCalled();
     });
 
@@ -164,10 +147,6 @@ describe('MDC-based Chip Remove', () => {
       dispatchKeyboardEvent(buttonElement, 'keydown', TAB);
       fixture.detectChanges();
 
-      const fakeEvent = createFakeEvent('transitionend');
-      (fakeEvent as any).propertyName = 'width';
-      chipNativeElement.dispatchEvent(fakeEvent);
-
       expect(testChip.didRemove).not.toHaveBeenCalled();
     });
 

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

@@ -2,7 +2,6 @@ import {Directionality} from '@angular/cdk/bidi';
 import {BACKSPACE, DELETE, RIGHT_ARROW, ENTER} from '@angular/cdk/keycodes';
 import {
   createKeyboardEvent,
-  createFakeEvent,
   dispatchEvent,
   dispatchFakeEvent,
 } from '@angular/cdk/testing/private';
@@ -97,10 +96,6 @@ describe('MDC-based Row Chips', () => {
         chipInstance.remove();
         fixture.detectChanges();
 
-        const fakeEvent = createFakeEvent('transitionend');
-        (fakeEvent as any).propertyName = 'width';
-        chipNativeElement.dispatchEvent(fakeEvent);
-
         expect(testComponent.chipRemove).toHaveBeenCalledWith({chip: chipInstance});
       });
 
@@ -127,10 +122,6 @@ describe('MDC-based Row Chips', () => {
           chipInstance._keydown(DELETE_EVENT);
           fixture.detectChanges();
 
-          const fakeEvent = createFakeEvent('transitionend');
-          (fakeEvent as any).propertyName = 'width';
-          chipNativeElement.dispatchEvent(fakeEvent);
-
           expect(testComponent.chipRemove).toHaveBeenCalled();
         });
 
@@ -142,10 +133,6 @@ describe('MDC-based Row Chips', () => {
           chipInstance._keydown(BACKSPACE_EVENT);
           fixture.detectChanges();
 
-          const fakeEvent = createFakeEvent('transitionend');
-          (fakeEvent as any).propertyName = 'width';
-          chipNativeElement.dispatchEvent(fakeEvent);
-
           expect(testComponent.chipRemove).toHaveBeenCalled();
         });
 
@@ -157,10 +144,6 @@ describe('MDC-based Row Chips', () => {
           removeIconInstance.interaction.next(ARROW_KEY_EVENT);
           fixture.detectChanges();
 
-          const fakeEvent = createFakeEvent('transitionend');
-          (fakeEvent as any).propertyName = 'width';
-          chipNativeElement.dispatchEvent(fakeEvent);
-
           expect(testComponent.chipRemove).not.toHaveBeenCalled();
         });
       });
@@ -179,10 +162,6 @@ describe('MDC-based Row Chips', () => {
           chipInstance._keydown(DELETE_EVENT);
           fixture.detectChanges();
 
-          const fakeEvent = createFakeEvent('transitionend');
-          (fakeEvent as any).propertyName = 'width';
-          chipNativeElement.dispatchEvent(fakeEvent);
-
           expect(testComponent.chipRemove).not.toHaveBeenCalled();
         });
 
@@ -195,10 +174,6 @@ describe('MDC-based Row Chips', () => {
           chipInstance._keydown(BACKSPACE_EVENT);
           fixture.detectChanges();
 
-          const fakeEvent = createFakeEvent('transitionend');
-          (fakeEvent as any).propertyName = 'width';
-          chipNativeElement.dispatchEvent(fakeEvent);
-
           expect(testComponent.chipRemove).not.toHaveBeenCalled();
         });
       });

src/material-experimental/mdc-chips/chip-row.ts

@@ -99,6 +99,12 @@ export class MatChipRow extends MatChip implements AfterContentInit, AfterViewIn
   /** The focusable grid cells for this row. Implemented as part of GridKeyManagerRow. */
   cells!: HTMLElement[];
 
+  /**
+   * Timeout used to give some time between `focusin` and `focusout`
+   * in order to determine whether focus has left the chip.
+   */
+  private _focusoutTimeout: number | null;
+
   constructor(
     @Inject(DOCUMENT) private readonly _document: any,
     changeDetectorRef: ChangeDetectorRef,
@@ -153,19 +159,25 @@ export class MatChipRow extends MatChip implements AfterContentInit, AfterViewIn
    * to the other gridcell.
    */
   _focusout(event: FocusEvent) {
-    this._hasFocusInternal = false;
+    if (this._focusoutTimeout) {
+      clearTimeout(this._focusoutTimeout);
+    }
+
     // Wait to see if focus moves to the other gridcell
-    setTimeout(() => {
-      if (this._hasFocus()) {
-        return;
-      }
+    this._focusoutTimeout = setTimeout(() => {
+      this._hasFocusInternal = false;
       this._onBlur.next({chip: this});
       this._handleInteraction(event);
     });
   }
 
   /** Records that the chip has focus when one of the gridcells is focused. */
   _focusin(event: FocusEvent) {
+    if (this._focusoutTimeout) {
+      clearTimeout(this._focusoutTimeout);
+      this._focusoutTimeout = null;
+    }
+
     this._hasFocusInternal = true;
     this._handleInteraction(event);
   }

src/material-experimental/mdc-chips/chip.spec.ts

@@ -1,5 +1,4 @@
 import {Directionality} from '@angular/cdk/bidi';
-import {createFakeEvent} from '@angular/cdk/testing/private';
 import {Component, DebugElement, ViewChild} from '@angular/core';
 import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing';
 import {MatRipple} from '@angular/material-experimental/mdc-core';
@@ -135,24 +134,9 @@ describe('MDC-based MatChip', () => {
       chipInstance.remove();
       fixture.detectChanges();
 
-      const fakeEvent = createFakeEvent('transitionend');
-      (fakeEvent as any).propertyName = 'width';
-      chipNativeElement.dispatchEvent(fakeEvent);
-
       expect(testComponent.chipRemove).toHaveBeenCalledWith({chip: chipInstance});
     });
 
-    it('should make the chip non-focusable when it is removed', () => {
-      chipInstance.remove();
-      fixture.detectChanges();
-
-      const fakeEvent = createFakeEvent('transitionend');
-      (fakeEvent as any).propertyName = 'width';
-      chipNativeElement.dispatchEvent(fakeEvent);
-
-      expect(chipNativeElement.style.display).toBe('none');
-    });
-
     it('should be able to disable ripples with the `[rippleDisabled]` input', () => {
       expect(chipRippleInstance.disabled).toBe(false, 'Expected chip ripples to be enabled.');
 

src/material-experimental/mdc-chips/chip.ts

@@ -231,9 +231,6 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
   /** The unstyled chip selector for this component. */
   protected basicChipAttrName = 'mat-basic-chip';
 
-  /** Subject that emits when the component has been destroyed. */
-  protected _destroyed = new Subject<void>();
-
   /** The chip's leading icon. */
   @ContentChild(MAT_CHIP_AVATAR) leadingIcon: MatChipAvatar;
 
@@ -278,15 +275,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
     notifyNavigation: () => this._notifyNavigation(),
     notifyTrailingIconInteraction: () =>
         this.removeIconInteraction.emit(this.id),
-    notifyRemoval:
-        () => {
-          this.removed.emit({chip: this});
-
-          // When MDC removes a chip it just transitions it to `width: 0px`
-          // which means that it's still in the DOM and it's still focusable.
-          // Make it `display: none` so users can't tab into it.
-          this._elementRef.nativeElement.style.display = 'none';
-        },
+    notifyRemoval: () => this.remove(),
     notifyEditStart:
         () => {
           this._onEditStart();
@@ -375,24 +364,17 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
 
   ngOnDestroy() {
     this.destroyed.emit({chip: this});
-    this._destroyed.next();
-    this._destroyed.complete();
     this._chipFoundation.destroy();
   }
 
   /** Sets up the remove icon chip foundation, and subscribes to remove icon events. */
-  _initRemoveIcon() {
+  private _initRemoveIcon() {
     if (this.removeIcon) {
       this._chipFoundation.setShouldRemoveOnTrailingIconClick(true);
-      this._listenToRemoveIconInteraction();
       this.removeIcon.disabled = this.disabled;
-    }
-  }
 
-  /** Handles interaction with the remove icon. */
-  _listenToRemoveIconInteraction() {
-    this.removeIcon.interaction
-        .pipe(takeUntil(this._destroyed))
+      this.removeIcon.interaction
+        .pipe(takeUntil(this.destroyed))
         .subscribe(event => {
           // The MDC chip foundation calls stopPropagation() for any trailing icon interaction
           // event, even ones it doesn't handle, so we want to avoid passing it keyboard events
@@ -405,7 +387,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
             return;
           }
 
-          this._chipFoundation.handleTrailingActionInteraction();
+          this.remove();
 
           if (isKeyboardEvent && !hasModifierKey(event as KeyboardEvent)) {
             const keyCode = (event as KeyboardEvent).keyCode;
@@ -416,6 +398,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
             }
           }
         });
+    }
   }
 
   /**
@@ -425,7 +408,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
    */
   remove(): void {
     if (this.removable) {
-      this._chipFoundation.beginExit();
+      this.removed.emit({chip: this});
     }
   }
 

src/material-experimental/mdc-chips/chips.scss

@@ -11,14 +11,8 @@
   cursor: default;
 
   &._mat-animation-noopable {
-    // MDC's chip removal works by toggling a class on the chip, waiting for its transitions
-    // to finish and emitting the remove event at the end. The problem is that if our animations
-    // were disabled via the `NoopAnimationsModule`, the element won't have a transition and
-    // `transitionend` won't fire. We work around the issue by assigning a very short transition.
-    transition-duration: 1ms;
-
-    // Disables the chip enter animation.
     animation: none;
+    transition: none;
 
     .mdc-chip__checkmark-svg {
       transition: none;

src/material-experimental/mdc-chips/testing/chip-harness.ts

@@ -43,9 +43,6 @@ export class MatChipHarness extends ComponentHarness {
   async remove(): Promise<void> {
     const hostEl = await this.host();
     await hostEl.sendKeys(TestKey.DELETE);
-
-    // @breaking-change 12.0.0 Remove non-null assertion from `dispatchEvent`.
-    await hostEl.dispatchEvent!('transitionend', {propertyName: 'width'});
   }
 
   /**