fix(focus-monitor): don't null-out focus until after event is finished

with capture & bubble

Author: mmalerba

src/cdk/a11y/focus-monitor/focus-monitor.spec.ts

@@ -6,10 +6,10 @@ import {
   patchElementFocus,
 } from '@angular/cdk/testing';
 import {Component} from '@angular/core';
-import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
+import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
-import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
 import {A11yModule} from '../index';
+import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
 
 
 describe('FocusMonitor', () => {
@@ -224,6 +224,7 @@ describe('cdkMonitorFocus', () => {
         ButtonWithFocusClasses,
         ComplexComponentWithMonitorElementFocus,
         ComplexComponentWithMonitorSubtreeFocus,
+        ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus,
       ],
     }).compileComponents();
   });
@@ -391,6 +392,36 @@ describe('cdkMonitorFocus', () => {
       expect(parentElement.classList.length).toBe(2, 'button should have exactly 2 focus classes');
     }));
   });
+
+  describe('complex component with cdkMonitorSubtreeFocus and cdkMonitorElementFocus', () => {
+    let fixture: ComponentFixture<ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus>;
+    let parentElement: HTMLElement;
+    let childElement: HTMLElement;
+    let focusMonitor: FocusMonitor;
+
+    beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => {
+      focusMonitor = fm;
+      fixture =
+          TestBed.createComponent(ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus);
+      fixture.detectChanges();
+
+      parentElement = fixture.debugElement.query(By.css('div')).nativeElement;
+      childElement = fixture.debugElement.query(By.css('button')).nativeElement;
+
+      patchElementFocus(parentElement);
+      patchElementFocus(childElement);
+    }));
+
+    it('should add keyboard focus classes on both elements when child is focused via keyboard',
+        fakeAsync(() => {
+          focusMonitor.focusVia(childElement, 'keyboard');
+          fixture.detectChanges();
+          flush();
+
+          expect(parentElement.classList).toContain('cdk-keyboard-focused');
+          expect(childElement.classList).toContain('cdk-keyboard-focused');
+        }));
+  });
 });
 
 
@@ -418,3 +449,8 @@ class ComplexComponentWithMonitorElementFocus {}
   template: `<div tabindex="0" cdkMonitorSubtreeFocus><button></button></div>`
 })
 class ComplexComponentWithMonitorSubtreeFocus {}
+
+@Component({
+  template: `<div cdkMonitorSubtreeFocus><button cdkMonitorElementFocus></button></div>`
+})
+class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {}

src/cdk/a11y/focus-monitor/focus-monitor.ts

@@ -18,7 +18,7 @@ import {
   Output,
   SkipSelf,
 } from '@angular/core';
-import {of as observableOf, Observable, Subject, Subscription} from 'rxjs';
+import {Observable, of as observableOf, Subject, Subscription} from 'rxjs';
 
 
 // This is the value used by AngularJS Material. Through trial and error (on iPhone 6S) they found
@@ -311,7 +311,10 @@ export class FocusMonitor implements OnDestroy {
     this._setClasses(element, this._origin);
     elementInfo.subject.next(this._origin);
     this._lastFocusOrigin = this._origin;
-    this._origin = null;
+
+    // Null-out the origin after a setTimeout. This allows the full capture/bubble cycle to complete
+    // for the current event before nulling it.
+    setTimeout(() => this._origin = null);
   }
 
   /**

src/demo-app/focus-origin/focus-origin-demo.html

@@ -19,3 +19,7 @@
   <p>div with subtree focus monitored</p>
   <button>1</button><button>2</button>
 </div>
+
+<div class="demo-focusable" cdkMonitorSubtreeFocus>
+  <mat-checkbox>Focus ring should show on keyboard focus</mat-checkbox>
+</div>

src/lib/button-toggle/button-toggle.spec.ts

@@ -1,13 +1,13 @@
-import {fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
 import {dispatchMouseEvent} from '@angular/cdk/testing';
-import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
-import {Component, DebugElement, ViewChild, ViewChildren, QueryList} from '@angular/core';
+import {Component, DebugElement, QueryList, ViewChild, ViewChildren} from '@angular/core';
+import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
+import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
 import {By} from '@angular/platform-browser';
 import {
-  MatButtonToggleGroup,
-  MatButtonToggleGroupMultiple,
   MatButtonToggle,
   MatButtonToggleChange,
+  MatButtonToggleGroup,
+  MatButtonToggleGroupMultiple,
   MatButtonToggleModule,
 } from './index';
 
@@ -574,13 +574,14 @@ describe('MatButtonToggle without forms', () => {
 
     it('should toggle when clicked', fakeAsync(() => {
       buttonToggleLabelElement.click();
-
       fixture.detectChanges();
+      flush();
 
       expect(buttonToggleInstance.checked).toBe(true);
 
       buttonToggleLabelElement.click();
       fixture.detectChanges();
+      flush();
 
       expect(buttonToggleInstance.checked).toBe(false);
     }));

src/lib/datepicker/datepicker.spec.ts

@@ -1,11 +1,11 @@
 import {ENTER, ESCAPE, RIGHT_ARROW, UP_ARROW} from '@angular/cdk/keycodes';
 import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
 import {
+  createKeyboardEvent,
+  dispatchEvent,
   dispatchFakeEvent,
   dispatchKeyboardEvent,
   dispatchMouseEvent,
-  createKeyboardEvent,
-  dispatchEvent,
 } from '@angular/cdk/testing';
 import {Component, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, flush, inject, TestBed} from '@angular/core/testing';
@@ -23,12 +23,12 @@ import {
 import {MatFormField, MatFormFieldModule} from '@angular/material/form-field';
 import {By} from '@angular/platform-browser';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {Subject} from 'rxjs';
 import {MatInputModule} from '../input/index';
 import {MatDatepicker} from './datepicker';
 import {MatDatepickerInput} from './datepicker-input';
 import {MatDatepickerToggle} from './datepicker-toggle';
 import {MAT_DATEPICKER_SCROLL_STRATEGY, MatDatepickerIntl, MatDatepickerModule} from './index';
-import {Subject} from 'rxjs';
 
 describe('MatDatepicker', () => {
   const SUPPORTS_INTL = typeof Intl != 'undefined';
@@ -297,6 +297,7 @@ describe('MatDatepicker', () => {
 
         testComponent.datepicker.open();
         fixture.detectChanges();
+        flush();
 
         let ownedElementId = inputEl.getAttribute('aria-owns');
         expect(ownedElementId).not.toBeNull();
@@ -902,6 +903,7 @@ describe('MatDatepicker', () => {
         testComponent.formField.color = 'primary';
         testComponent.datepicker.open();
         fixture.detectChanges();
+        flush();
 
         let contentEl = document.querySelector('.mat-datepicker-content')!;
 
@@ -916,6 +918,7 @@ describe('MatDatepicker', () => {
 
         contentEl = document.querySelector('.mat-datepicker-content')!;
         fixture.detectChanges();
+        flush();
 
         expect(contentEl.classList).toContain('mat-warn');
         expect(contentEl.classList).not.toContain('mat-primary');
@@ -926,6 +929,7 @@ describe('MatDatepicker', () => {
         testComponent.formField.color = 'warn';
         testComponent.datepicker.open();
         fixture.detectChanges();
+        flush();
 
         const contentEl = document.querySelector('.mat-datepicker-content')!;
 

src/lib/slide-toggle/slide-toggle.spec.ts

@@ -1,14 +1,19 @@
+import {dispatchFakeEvent} from '@angular/cdk/testing';
 import {Component} from '@angular/core';
-import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
 import {
-  async, ComponentFixture, TestBed, fakeAsync, tick,
-  flushMicrotasks
+  async,
+  ComponentFixture,
+  fakeAsync,
+  flush,
+  flushMicrotasks,
+  TestBed,
+  tick
 } from '@angular/core/testing';
-import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
-import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
-import {TestGestureConfig} from '../slider/test-gesture-config';
-import {dispatchFakeEvent} from '@angular/cdk/testing';
+import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
 import {defaultRippleAnimationConfig} from '@angular/material/core';
+import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
+import {TestGestureConfig} from '../slider/test-gesture-config';
+import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
 
 describe('MatSlideToggle without forms', () => {
   let gestureConfig: TestGestureConfig;
@@ -723,6 +728,7 @@ describe('MatSlideToggle with forms', () => {
 
       labelElement.click();
       fixture.detectChanges();
+      flush();
 
       expect(slideToggle.checked)
         .toBe(false, 'Expected slide-toggle to be no longer checked after label click.');