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

This reverts commit 2d66ecb.

Author: mmalerba

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

@@ -12,6 +12,7 @@ import {
   TAB
 } from '@angular/cdk/keycodes';
 import {
+  createFakeEvent,
   createKeyboardEvent,
   dispatchEvent,
   dispatchFakeEvent,
@@ -215,7 +216,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();
@@ -601,13 +602,15 @@ 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;
     });
@@ -620,6 +623,12 @@ 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);
     });
@@ -762,10 +771,9 @@ 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');
     }));
 
@@ -779,6 +787,10 @@ 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: Math.max(this._keyManager.activeColumnIndex, 0)
+          column: this._keyManager.activeColumnIndex
         });
       } else {
         this.focus();

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

@@ -1,4 +1,5 @@
 import {
+  createFakeEvent,
   dispatchKeyboardEvent,
   createKeyboardEvent,
   dispatchEvent,
@@ -54,6 +55,18 @@ 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')!;
 
@@ -64,6 +77,10 @@ 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();
     });
 
@@ -147,6 +164,10 @@ 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,6 +2,7 @@ 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';
@@ -96,6 +97,10 @@ 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});
       });
 
@@ -122,6 +127,10 @@ 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();
         });
 
@@ -133,6 +142,10 @@ 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();
         });
 
@@ -144,6 +157,10 @@ 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();
         });
       });
@@ -162,6 +179,10 @@ 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();
         });
 
@@ -174,6 +195,10 @@ 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,12 +99,6 @@ 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,
@@ -159,25 +153,19 @@ export class MatChipRow extends MatChip implements AfterContentInit, AfterViewIn
    * to the other gridcell.
    */
   _focusout(event: FocusEvent) {
-    if (this._focusoutTimeout) {
-      clearTimeout(this._focusoutTimeout);
-    }
-
+    this._hasFocusInternal = false;
     // Wait to see if focus moves to the other gridcell
-    this._focusoutTimeout = setTimeout(() => {
-      this._hasFocusInternal = false;
+    setTimeout(() => {
+      if (this._hasFocus()) {
+        return;
+      }
       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,4 +1,5 @@
 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';
@@ -134,9 +135,24 @@ 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,6 +231,9 @@ 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;
 
@@ -275,7 +278,15 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
     notifyNavigation: () => this._notifyNavigation(),
     notifyTrailingIconInteraction: () =>
         this.removeIconInteraction.emit(this.id),
-    notifyRemoval: () => this.remove(),
+    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';
+        },
     notifyEditStart:
         () => {
           this._onEditStart();
@@ -364,17 +375,24 @@ 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. */
-  private _initRemoveIcon() {
+  _initRemoveIcon() {
     if (this.removeIcon) {
       this._chipFoundation.setShouldRemoveOnTrailingIconClick(true);
+      this._listenToRemoveIconInteraction();
       this.removeIcon.disabled = this.disabled;
+    }
+  }
 
-      this.removeIcon.interaction
-        .pipe(takeUntil(this.destroyed))
+  /** Handles interaction with the remove icon. */
+  _listenToRemoveIconInteraction() {
+    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
@@ -387,7 +405,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
             return;
           }
 
-          this.remove();
+          this._chipFoundation.handleTrailingActionInteraction();
 
           if (isKeyboardEvent && !hasModifierKey(event as KeyboardEvent)) {
             const keyCode = (event as KeyboardEvent).keyCode;
@@ -398,7 +416,6 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
             }
           }
         });
-    }
   }
 
   /**
@@ -408,7 +425,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
    */
   remove(): void {
     if (this.removable) {
-      this.removed.emit({chip: this});
+      this._chipFoundation.beginExit();
     }
   }
 

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

@@ -11,8 +11,14 @@
   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,6 +43,9 @@ 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'});
   }
 
   /**