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

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

* fixup! fix(material-experimental/mdc-chips): decouple removal from animation

Co-authored-by: Kristiyan Kostadinov <[email protected]>

Author: mmalerba

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

@@ -213,9 +213,6 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
   }
   protected _highlighted: boolean = false;
 
-  /** Emitted when the user interacts with the remove icon. */
-  @Output() removeIconInteraction = new EventEmitter<string>();
-
   /** Emitted when the user interacts with the chip. */
   @Output() interaction = new EventEmitter<string>();
 
@@ -231,9 +228,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;
 
@@ -276,17 +270,8 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
           // input.
         },
     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';
-        },
+    notifyTrailingIconInteraction: () => {},
+    notifyRemoval: () => this.remove(),
     notifyEditStart:
         () => {
           this._onEditStart();
@@ -375,24 +360,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 +383,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 +394,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
             }
           }
         });
+    }
   }
 
   /**
@@ -425,7 +404,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'});
   }
 
   /**