fix(material/chips): focus escape not working consistently

Fixes that the chip grid was using change detection to allow focus to escape on tab presses. That's unreliable, because change detection might not be executed immediately. These changes fix the issue by changing the DOM node directly.

Author: crisbeto

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

@@ -396,37 +396,37 @@ describe('MatChipGrid', () => {
 
           dispatchKeyboardEvent(firstNativeChip, 'keydown', TAB);
 
-          expect(chipGridInstance.tabIndex)
+          expect(chipGridNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to -1 temporarily.')
             .toBe(-1);
 
           flush();
 
-          expect(chipGridInstance.tabIndex)
+          expect(chipGridNativeElement.tabIndex)
             .withContext('Expected tabIndex to be reset back to 0')
             .toBe(0);
         }));
 
-        it(`should use user defined tabIndex`, fakeAsync(() => {
+        it('should use user defined tabIndex', fakeAsync(() => {
           chipGridInstance.tabIndex = 4;
           fixture.changeDetectorRef.markForCheck();
           fixture.detectChanges();
 
-          expect(chipGridInstance.tabIndex)
+          expect(chipGridNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to user defined value 4.')
             .toBe(4);
 
           let nativeChips = chipGridNativeElement.querySelectorAll('mat-chip-row');
           let firstNativeChip = nativeChips[0] as HTMLElement;
 
           dispatchKeyboardEvent(firstNativeChip, 'keydown', TAB);
-          expect(chipGridInstance.tabIndex)
+          expect(chipGridNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to -1 temporarily.')
             .toBe(-1);
 
           flush();
 
-          expect(chipGridInstance.tabIndex)
+          expect(chipGridNativeElement.tabIndex)
             .withContext('Expected tabIndex to be reset back to 4')
             .toBe(4);
         }));

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

@@ -367,13 +367,13 @@ describe('MatChipListbox', () => {
         it('should allow focus to escape when tabbing away', fakeAsync(() => {
           dispatchKeyboardEvent(chipListboxNativeElement, 'keydown', TAB);
 
-          expect(chipListboxInstance.tabIndex)
+          expect(chipListboxNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to -1 temporarily.')
             .toBe(-1);
 
           flush();
 
-          expect(chipListboxInstance.tabIndex)
+          expect(chipListboxNativeElement.tabIndex)
             .withContext('Expected tabIndex to be reset back to 0')
             .toBe(0);
         }));
@@ -384,19 +384,19 @@ describe('MatChipListbox', () => {
 
           fixture.detectChanges();
 
-          expect(chipListboxInstance.tabIndex)
+          expect(chipListboxNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to user defined value 4.')
             .toBe(4);
 
           dispatchKeyboardEvent(chipListboxNativeElement, 'keydown', TAB);
 
-          expect(chipListboxInstance.tabIndex)
+          expect(chipListboxNativeElement.tabIndex)
             .withContext('Expected tabIndex to be set to -1 temporarily.')
             .toBe(-1);
 
           flush();
 
-          expect(chipListboxInstance.tabIndex)
+          expect(chipListboxNativeElement.tabIndex)
             .withContext('Expected tabIndex to be reset back to 4')
             .toBe(4);
         }));

src/material/chips/chip-set.ts

@@ -192,17 +192,17 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
    * it back to the first chip, creating a focus trap, if it user tries to tab away.
    */
   protected _allowFocusEscape() {
-    if (this.tabIndex !== -1) {
-      const previousTabIndex = this.tabIndex;
-      this.tabIndex = -1;
-      this._changeDetectorRef.markForCheck();
+    const previous = this._elementRef.nativeElement.tabIndex;
+
+    if (previous !== -1) {
+      // Set the tabindex directly on the element, instead of going through
+      // the data binding, because we aren't guaranteed that change detection
+      // will run quickly enough to allow focus to escape.
+      this._elementRef.nativeElement.tabIndex = -1;
 
       // Note that this needs to be a `setTimeout`, because a `Promise.resolve`
       // doesn't allow enough time for the focus to escape.
-      setTimeout(() => {
-        this.tabIndex = previousTabIndex;
-        this._changeDetectorRef.markForCheck();
-      });
+      setTimeout(() => (this._elementRef.nativeElement.tabIndex = previous));
     }
   }