From c38ae56f9345c78ae02d33291d3bfeedb207b4a3 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 23 Mar 2018 14:43:35 -0700 Subject: [PATCH 1/3] fix(focus-monitor): reenter ngzone before emitting --- .../a11y/focus-monitor/focus-monitor.spec.ts | 54 ++++++++++++++++++- src/cdk/a11y/focus-monitor/focus-monitor.ts | 8 ++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts index cb1414ca9b22..474c36d34caf 100644 --- a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts +++ b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts @@ -5,7 +5,7 @@ import { dispatchMouseEvent, patchElementFocus, } from '@angular/cdk/testing'; -import {Component} from '@angular/core'; +import {Component, ElementRef, ViewChild} from '@angular/core'; import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {A11yModule} from '../index'; @@ -212,7 +212,6 @@ describe('FocusMonitor', () => { expect(buttonElement.classList.length).toBe(0, 'button should not have any focus classes'); })); - }); @@ -424,6 +423,39 @@ describe('cdkMonitorFocus', () => { }); }); +describe('FocusMonitor observable stream', () => { + let fixture: ComponentFixture; + let buttonElement: HTMLElement; + let focusMonitor: FocusMonitor; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [A11yModule], + declarations: [ + MonitoredElementRequiringChangeDetection, + ], + }).compileComponents(); + }); + + beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => { + fixture = TestBed.createComponent(MonitoredElementRequiringChangeDetection); + fixture.detectChanges(); + + buttonElement = fixture.componentInstance.button.nativeElement; + focusMonitor = fm; + + patchElementFocus(buttonElement); + })); + + it('should emit inside the NgZone', () => { + fixture.detectChanges(); + expect(buttonElement.innerText).toBe(''); + buttonElement.focus(); + fixture.detectChanges(); + expect(buttonElement.innerText).toBe('program'); + }); +}); + @Component({ template: `` @@ -454,3 +486,21 @@ class ComplexComponentWithMonitorSubtreeFocus {} template: `
` }) class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {} + +@Component({ + template: `` +}) +class MonitoredElementRequiringChangeDetection { + @ViewChild('b') button: ElementRef; + origin: string; + + constructor(private _focusMonitor: FocusMonitor) {} + + ngOnInit() { + this._focusMonitor.monitor(this.button.nativeElement).subscribe(o => this.origin = o || ''); + } + + ngOnDestroy() { + this._focusMonitor.stopMonitoring(this.button.nativeElement); + } +} diff --git a/src/cdk/a11y/focus-monitor/focus-monitor.ts b/src/cdk/a11y/focus-monitor/focus-monitor.ts index 7eb48efa0550..5628a307be65 100644 --- a/src/cdk/a11y/focus-monitor/focus-monitor.ts +++ b/src/cdk/a11y/focus-monitor/focus-monitor.ts @@ -314,7 +314,7 @@ export class FocusMonitor implements OnDestroy { } this._setClasses(element, origin); - elementInfo.subject.next(origin); + this._emitOrigin(elementInfo.subject, origin); this._lastFocusOrigin = origin; } @@ -334,7 +334,11 @@ export class FocusMonitor implements OnDestroy { } this._setClasses(element); - elementInfo.subject.next(null); + this._emitOrigin(elementInfo.subject, null); + } + + private _emitOrigin(subject: Subject, origin: FocusOrigin) { + this._ngZone.run(() => subject.next(origin)); } private _incrementMonitoredElementCount() { From 43bd51cc5ecc83dd9b01d8c358de9ceee50275a6 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 23 Mar 2018 15:42:42 -0700 Subject: [PATCH 2/3] fix tests --- .../a11y/focus-monitor/focus-monitor.spec.ts | 46 ++++++------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts index 474c36d34caf..33f8b100623d 100644 --- a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts +++ b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts @@ -5,7 +5,7 @@ import { dispatchMouseEvent, patchElementFocus, } from '@angular/cdk/testing'; -import {Component, ElementRef, ViewChild} from '@angular/core'; +import {Component, NgZone} from '@angular/core'; import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {A11yModule} from '../index'; @@ -223,7 +223,6 @@ describe('cdkMonitorFocus', () => { ButtonWithFocusClasses, ComplexComponentWithMonitorElementFocus, ComplexComponentWithMonitorSubtreeFocus, - ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus, ], }).compileComponents(); }); @@ -424,7 +423,7 @@ describe('cdkMonitorFocus', () => { }); describe('FocusMonitor observable stream', () => { - let fixture: ComponentFixture; + let fixture: ComponentFixture; let buttonElement: HTMLElement; let focusMonitor: FocusMonitor; @@ -432,28 +431,29 @@ describe('FocusMonitor observable stream', () => { TestBed.configureTestingModule({ imports: [A11yModule], declarations: [ - MonitoredElementRequiringChangeDetection, + PlainButton, ], }).compileComponents(); }); beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => { - fixture = TestBed.createComponent(MonitoredElementRequiringChangeDetection); - fixture.detectChanges(); - - buttonElement = fixture.componentInstance.button.nativeElement; + fixture = TestBed.createComponent(PlainButton); focusMonitor = fm; - + fixture.detectChanges(); + buttonElement = fixture.debugElement.nativeElement.querySelector('button'); patchElementFocus(buttonElement); })); - it('should emit inside the NgZone', () => { - fixture.detectChanges(); - expect(buttonElement.innerText).toBe(''); + it('should emit inside the NgZone', fakeAsync(() => { + const spy = jasmine.createSpy('zone spy'); + focusMonitor.monitor(buttonElement).subscribe(() => spy(NgZone.isInAngularZone())); + expect(spy).not.toHaveBeenCalled(); + buttonElement.focus(); fixture.detectChanges(); - expect(buttonElement.innerText).toBe('program'); - }); + tick(); + expect(spy).toHaveBeenCalledWith(true); + })); }); @@ -486,21 +486,3 @@ class ComplexComponentWithMonitorSubtreeFocus {} template: `
` }) class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {} - -@Component({ - template: `` -}) -class MonitoredElementRequiringChangeDetection { - @ViewChild('b') button: ElementRef; - origin: string; - - constructor(private _focusMonitor: FocusMonitor) {} - - ngOnInit() { - this._focusMonitor.monitor(this.button.nativeElement).subscribe(o => this.origin = o || ''); - } - - ngOnDestroy() { - this._focusMonitor.stopMonitoring(this.button.nativeElement); - } -} From 94341f1e62f9be913e5050fac5c8e838543a82fa Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 4 May 2018 10:55:47 -0700 Subject: [PATCH 3/3] fix bad rebase --- src/cdk/a11y/focus-monitor/focus-monitor.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts index 33f8b100623d..331e240434cf 100644 --- a/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts +++ b/src/cdk/a11y/focus-monitor/focus-monitor.spec.ts @@ -223,6 +223,7 @@ describe('cdkMonitorFocus', () => { ButtonWithFocusClasses, ComplexComponentWithMonitorElementFocus, ComplexComponentWithMonitorSubtreeFocus, + ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus, ], }).compileComponents(); }); @@ -392,7 +393,7 @@ describe('cdkMonitorFocus', () => { }); describe('complex component with cdkMonitorSubtreeFocus and cdkMonitorElementFocus', () => { - let fixture: ComponentFixture; + let fixture: ComponentFixture; let parentElement: HTMLElement; let childElement: HTMLElement; let focusMonitor: FocusMonitor; @@ -400,7 +401,7 @@ describe('cdkMonitorFocus', () => { beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => { focusMonitor = fm; fixture = - TestBed.createComponent(ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus); + TestBed.createComponent(ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus); fixture.detectChanges(); parentElement = fixture.debugElement.query(By.css('div')).nativeElement; @@ -485,4 +486,4 @@ class ComplexComponentWithMonitorSubtreeFocus {} @Component({ template: `
` }) -class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {} +class ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus {}