fix(chips): focus not being restored correctly on chip removal when inside component with animations

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes #12374.

Author: crisbeto

src/demo-app/chips/chips-demo.ts

@@ -9,6 +9,7 @@
 import {COMMA, ENTER} from '@angular/cdk/keycodes';
 import {Component} from '@angular/core';
 import {MatChipInputEvent, ThemePalette} from '@angular/material';
+import {trigger, transition, style, animate} from '@angular/animations';
 
 
 export interface Person {
@@ -24,7 +25,17 @@ export interface DemoColor {
   moduleId: module.id,
   selector: 'chips-demo',
   templateUrl: 'chips-demo.html',
-  styleUrls: ['chips-demo.css']
+  styleUrls: ['chips-demo.css'],
+  animations: [
+    trigger(
+      'oeuoeuoeu', [
+        transition(':leave', [
+          style({opacity: 0}),
+          animate('500ms', style({opacity: 1}))
+        ])
+      ]
+    )
+  ]
 })
 export class ChipsDemo {
   tabIndex = 0;

src/lib/chips/chip-list.spec.ts

@@ -1,7 +1,12 @@
 import {FocusKeyManager} from '@angular/cdk/a11y';
 import {Directionality, Direction} from '@angular/cdk/bidi';
 import {BACKSPACE, DELETE, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, TAB} from '@angular/cdk/keycodes';
-import {createKeyboardEvent, dispatchFakeEvent, dispatchKeyboardEvent} from '@angular/cdk/testing';
+import {
+  createKeyboardEvent,
+  dispatchFakeEvent,
+  dispatchKeyboardEvent,
+  MockNgZone,
+} from '@angular/cdk/testing';
 import {
   Component,
   DebugElement,
@@ -10,16 +15,18 @@ import {
   ViewChildren,
   Type,
   Provider,
+  NgZone,
 } from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
 import {FormControl, FormsModule, NgForm, ReactiveFormsModule, Validators} from '@angular/forms';
 import {MatFormFieldModule} from '@angular/material/form-field';
 import {By} from '@angular/platform-browser';
-import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {NoopAnimationsModule, BrowserAnimationsModule} from '@angular/platform-browser/animations';
 import {MatInputModule} from '../input/index';
 import {MatChip} from './chip';
 import {MatChipInputEvent} from './chip-input';
 import {MatChipList, MatChipsModule} from './index';
+import {trigger, transition, style, animate} from '@angular/animations';
 
 
 describe('MatChipList', () => {
@@ -30,6 +37,7 @@ describe('MatChipList', () => {
   let testComponent: StandardChipList;
   let chips: QueryList<any>;
   let manager: FocusKeyManager<MatChip>;
+  let zone: MockNgZone;
 
   describe('StandardChipList', () => {
     describe('basic behaviors', () => {
@@ -154,6 +162,7 @@ describe('MatChipList', () => {
           // Focus and blur the middle item
           midItem.focus();
           midItem._blur();
+          zone.simulateZoneExit();
 
           // Destroy the middle item
           testComponent.remove = 2;
@@ -162,6 +171,32 @@ describe('MatChipList', () => {
           // Should not have focus
           expect(chipListInstance._keyManager.activeItemIndex).toEqual(-1);
         });
+
+        it('should move focus to the last chip when the focused chip was deleted inside a' +
+          'component with animations', fakeAsync(() => {
+            fixture.destroy();
+            TestBed.resetTestingModule();
+            fixture = createComponent(StandardChipListWithAnimations, [], BrowserAnimationsModule);
+            fixture.detectChanges();
+
+            chipListDebugElement = fixture.debugElement.query(By.directive(MatChipList));
+            chipListNativeElement = chipListDebugElement.nativeElement;
+            chipListInstance = chipListDebugElement.componentInstance;
+            testComponent = fixture.debugElement.componentInstance;
+            chips = chipListInstance.chips;
+
+            chips.last.focus();
+            fixture.detectChanges();
+
+            expect(chipListInstance._keyManager.activeItemIndex).toBe(chips.length - 1);
+
+            dispatchKeyboardEvent(chips.last._elementRef.nativeElement, 'keydown', BACKSPACE);
+            fixture.detectChanges();
+            tick(500);
+
+            expect(chipListInstance._keyManager.activeItemIndex).toBe(chips.length - 1);
+          }));
+
       });
     });
 
@@ -1018,18 +1053,23 @@ describe('MatChipList', () => {
     });
   });
 
-  function createComponent<T>(component: Type<T>, providers: Provider[] = []): ComponentFixture<T> {
+  function createComponent<T>(component: Type<T>, providers: Provider[] = [], animationsModule:
+      Type<NoopAnimationsModule> | Type<BrowserAnimationsModule> = NoopAnimationsModule):
+          ComponentFixture<T> {
     TestBed.configureTestingModule({
       imports: [
         FormsModule,
         ReactiveFormsModule,
         MatChipsModule,
         MatFormFieldModule,
         MatInputModule,
-        NoopAnimationsModule,
+        animationsModule,
       ],
       declarations: [component],
-      providers
+      providers: [
+        {provide: NgZone, useFactory: () => zone = new MockNgZone()},
+        ...providers
+      ]
     }).compileComponents();
 
     return TestBed.createComponent<T>(component);
@@ -1293,3 +1333,33 @@ class ChipListWithFormErrorMessages {
   @ViewChild('form') form: NgForm;
   formControl = new FormControl('', Validators.required);
 }
+
+
+@Component({
+  template: `
+    <mat-chip-list>
+      <mat-chip *ngFor="let i of numbers" (removed)="remove(i)">{{i}}</mat-chip>
+    </mat-chip-list>`,
+  animations: [
+    // For the case we're testing this animation doesn't
+    // have to be used anywhere, it just has to be defined.
+    trigger('dummyAnimation', [
+      transition(':leave', [
+        style({opacity: 0}),
+        animate('500ms', style({opacity: 1}))
+      ])
+    ])
+  ]
+})
+class StandardChipListWithAnimations {
+  numbers = [0, 1, 2, 3, 4];
+
+  remove(item: number): void {
+    const index = this.numbers.indexOf(item);
+
+    if (index > -1) {
+      this.numbers.splice(index, 1);
+    }
+  }
+}
+

src/lib/chips/chip.ts

@@ -37,6 +37,7 @@ import {
   RippleTarget
 } from '@angular/material/core';
 import {Subject} from 'rxjs';
+import { take } from 'rxjs/operators';
 
 
 /** Represents an event fired on an individual `mat-chip`. */
@@ -218,14 +219,14 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
   }
 
   constructor(public _elementRef: ElementRef,
-              ngZone: NgZone,
+              private _ngZone: NgZone,
               platform: Platform,
               @Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS) globalOptions: RippleGlobalOptions) {
     super(_elementRef);
 
     this._addHostClassName();
 
-    this._chipRipple = new RippleRenderer(this, ngZone, _elementRef, platform);
+    this._chipRipple = new RippleRenderer(this, _ngZone, _elementRef, platform);
     this._chipRipple.setupTriggerEvents(_elementRef.nativeElement);
 
     if (globalOptions) {
@@ -359,7 +360,15 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
   }
 
   _blur(): void {
-    this._hasFocus = false;
+    // When animations are enabled, Angular may end up removing the chip from the DOM a little
+    // earlier than usual, causing it to be blurred and throwing off the logic in the chip list
+    // that moves focus not the next item. To work around the issue, we defer marking the chip
+    // as not focused until the next time the zone stabilizes.
+    this._ngZone.onStable
+      .asObservable()
+      .pipe(take(1))
+      .subscribe(() => this._hasFocus = false);
+
     this._onBlur.next({chip: this});
   }
 }