From 2240b6c5a80c4ddb1b0fb9b6183ef4d9d71952c8 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 31 Mar 2017 19:42:35 +0200 Subject: [PATCH] refactor(focus-origin-monitor): rename unmonitor to stopMonitoring (#3783) * Renames the `unmonitor` function to `stopMonitoring`. * Removes unnecessary code that keeps track of the `FocusOriginMonitor` subscription. After calling `stopMonitoring` the observable will complete. --- src/lib/button/button.ts | 2 +- src/lib/checkbox/checkbox.spec.ts | 18 +++---------- src/lib/checkbox/checkbox.ts | 7 +---- .../core/style/focus-origin-monitor.spec.ts | 4 +-- src/lib/core/style/focus-origin-monitor.ts | 4 +-- src/lib/radio/radio.spec.ts | 27 +++++++------------ src/lib/radio/radio.ts | 7 +---- src/lib/slider/slider.ts | 2 +- 8 files changed, 21 insertions(+), 50 deletions(-) diff --git a/src/lib/button/button.ts b/src/lib/button/button.ts index 411e04c94bf..9a96d575099 100644 --- a/src/lib/button/button.ts +++ b/src/lib/button/button.ts @@ -125,7 +125,7 @@ export class MdButton implements OnDestroy { } ngOnDestroy() { - this._focusOriginMonitor.unmonitor(this._elementRef.nativeElement); + this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement); } /** The color of the button. Can be `primary`, `accent`, or `warn`. */ diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts index 29ddc7e85e6..48a4c1a15ac 100644 --- a/src/lib/checkbox/checkbox.spec.ts +++ b/src/lib/checkbox/checkbox.spec.ts @@ -13,22 +13,11 @@ import {MdCheckbox, MdCheckboxChange, MdCheckboxModule} from './index'; import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler'; import {dispatchFakeEvent} from '../core/testing/dispatch-events'; -import {FocusOriginMonitor, FocusOrigin} from '../core'; import {RIPPLE_FADE_IN_DURATION, RIPPLE_FADE_OUT_DURATION} from '../core/ripple/ripple-renderer'; -import {Subject} from 'rxjs/Subject'; describe('MdCheckbox', () => { let fixture: ComponentFixture; - let fakeFocusOriginMonitorSubject: Subject = new Subject(); - let fakeFocusOriginMonitor = { - monitor: () => fakeFocusOriginMonitorSubject.asObservable(), - unmonitor: () => {}, - focusVia: (element: HTMLElement, renderer: any, focusOrigin: FocusOrigin) => { - element.focus(); - fakeFocusOriginMonitorSubject.next(focusOrigin); - } - }; beforeEach(async(() => { TestBed.configureTestingModule({ @@ -45,8 +34,7 @@ describe('MdCheckbox', () => { CheckboxWithFormControl, ], providers: [ - {provide: ViewportRuler, useClass: FakeViewportRuler}, - {provide: FocusOriginMonitor, useValue: fakeFocusOriginMonitor} + {provide: ViewportRuler, useClass: FakeViewportRuler} ] }); @@ -356,7 +344,9 @@ describe('MdCheckbox', () => { expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length) .toBe(0, 'Expected no ripples on load.'); - fakeFocusOriginMonitorSubject.next('keyboard'); + dispatchFakeEvent(inputElement, 'keydown'); + dispatchFakeEvent(inputElement, 'focus'); + tick(RIPPLE_FADE_IN_DURATION); expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length) diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts index e7ebf7c3942..ae9e0b00f09 100644 --- a/src/lib/checkbox/checkbox.ts +++ b/src/lib/checkbox/checkbox.ts @@ -206,12 +206,7 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro } ngOnDestroy() { - this._focusOriginMonitor.unmonitor(this._inputElement.nativeElement); - - if (this._focusedSubscription) { - this._focusedSubscription.unsubscribe(); - this._focusedSubscription = null; - } + this._focusOriginMonitor.stopMonitoring(this._inputElement.nativeElement); } /** diff --git a/src/lib/core/style/focus-origin-monitor.spec.ts b/src/lib/core/style/focus-origin-monitor.spec.ts index 7f17f3ea7cb..fb4603be5dc 100644 --- a/src/lib/core/style/focus-origin-monitor.spec.ts +++ b/src/lib/core/style/focus-origin-monitor.spec.ts @@ -218,7 +218,7 @@ describe('FocusOriginMonitor', () => { }, 0); })); - it('should remove classes on unmonitor', async(() => { + it('should remove classes on stopMonitoring', async(() => { buttonElement.focus(); fixture.detectChanges(); @@ -228,7 +228,7 @@ describe('FocusOriginMonitor', () => { expect(buttonElement.classList.length) .toBe(2, 'button should have exactly 2 focus classes'); - focusOriginMonitor.unmonitor(buttonElement); + focusOriginMonitor.stopMonitoring(buttonElement); fixture.detectChanges(); expect(buttonElement.classList.length).toBe(0, 'button should not have any focus classes'); diff --git a/src/lib/core/style/focus-origin-monitor.ts b/src/lib/core/style/focus-origin-monitor.ts index 42d83bb012c..7d9da7a6586 100644 --- a/src/lib/core/style/focus-origin-monitor.ts +++ b/src/lib/core/style/focus-origin-monitor.ts @@ -101,7 +101,7 @@ export class FocusOriginMonitor { * Stops monitoring an element and removes all focus classes. * @param element The element to stop monitoring. */ - unmonitor(element: Element): void { + stopMonitoring(element: Element): void { let elementInfo = this._elementInfo.get(element); if (elementInfo) { @@ -296,7 +296,7 @@ export class CdkMonitorFocus implements OnDestroy { } ngOnDestroy() { - this._focusOriginMonitor.unmonitor(this._elementRef.nativeElement); + this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement); } } diff --git a/src/lib/radio/radio.spec.ts b/src/lib/radio/radio.spec.ts index 238949c9488..e1cc41333fc 100644 --- a/src/lib/radio/radio.spec.ts +++ b/src/lib/radio/radio.spec.ts @@ -6,21 +6,10 @@ import {MdRadioGroup, MdRadioButton, MdRadioChange, MdRadioModule} from './index import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; import {FakeViewportRuler} from '../core/overlay/position/fake-viewport-ruler'; import {dispatchFakeEvent} from '../core/testing/dispatch-events'; -import {FocusOriginMonitor, FocusOrigin} from '../core'; import {RIPPLE_FADE_IN_DURATION, RIPPLE_FADE_OUT_DURATION} from '../core/ripple/ripple-renderer'; -import {Subject} from 'rxjs/Subject'; describe('MdRadio', () => { - let fakeFocusOriginMonitorStream = new Subject(); - let fakeFocusOriginMonitor = { - monitor: () => fakeFocusOriginMonitorStream.asObservable(), - unmonitor: () => {}, - focusVia: (element: HTMLElement, renderer: any, origin: FocusOrigin) => { - element.focus(); - fakeFocusOriginMonitorStream.next(origin); - } - }; beforeEach(async(() => { TestBed.configureTestingModule({ @@ -32,8 +21,7 @@ describe('MdRadio', () => { StandaloneRadioButtons, ], providers: [ - {provide: ViewportRuler, useClass: FakeViewportRuler}, - {provide: FocusOriginMonitor, useValue: fakeFocusOriginMonitor} + {provide: ViewportRuler, useClass: FakeViewportRuler} ] }); @@ -47,6 +35,7 @@ describe('MdRadio', () => { let radioDebugElements: DebugElement[]; let radioNativeElements: HTMLElement[]; let radioLabelElements: HTMLLabelElement[]; + let radioInputElements: HTMLInputElement[]; let groupInstance: MdRadioGroup; let radioInstances: MdRadioButton[]; let testComponent: RadiosInsideRadioGroup; @@ -67,6 +56,8 @@ describe('MdRadio', () => { radioLabelElements = radioDebugElements .map(debugEl => debugEl.query(By.css('label')).nativeElement); + radioInputElements = radioDebugElements + .map(debugEl => debugEl.query(By.css('input')).nativeElement); })); it('should set individual radio names based on the group name', () => { @@ -140,9 +131,7 @@ describe('MdRadio', () => { }); it('should check a radio upon interaction with the underlying native radio button', () => { - let nativeRadioInput = radioNativeElements[0].querySelector('input'); - - nativeRadioInput.click(); + radioInputElements[0].click(); fixture.detectChanges(); expect(radioInstances[0].checked).toBe(true); @@ -194,13 +183,15 @@ describe('MdRadio', () => { expect(radioNativeElements[0].querySelectorAll('.mat-ripple-element').length) .toBe(0, 'Expected no ripples on init.'); - fakeFocusOriginMonitorStream.next('keyboard'); + dispatchFakeEvent(radioInputElements[0], 'keydown'); + dispatchFakeEvent(radioInputElements[0], 'focus'); + tick(RIPPLE_FADE_IN_DURATION); expect(radioNativeElements[0].querySelectorAll('.mat-ripple-element').length) .toBe(1, 'Expected one ripple after keyboard focus.'); - dispatchFakeEvent(radioNativeElements[0].querySelector('input'), 'blur'); + dispatchFakeEvent(radioInputElements[0], 'blur'); tick(RIPPLE_FADE_OUT_DURATION); expect(radioNativeElements[0].querySelectorAll('.mat-ripple-element').length) diff --git a/src/lib/radio/radio.ts b/src/lib/radio/radio.ts index ddad9a8ed8c..7e72e6c2d03 100644 --- a/src/lib/radio/radio.ts +++ b/src/lib/radio/radio.ts @@ -456,12 +456,7 @@ export class MdRadioButton implements OnInit, AfterViewInit, OnDestroy { } ngOnDestroy() { - this._focusOriginMonitor.unmonitor(this._inputElement.nativeElement); - - if (this._focusOriginMonitorSubscription) { - this._focusOriginMonitorSubscription.unsubscribe(); - this._focusOriginMonitorSubscription = null; - } + this._focusOriginMonitor.stopMonitoring(this._inputElement.nativeElement); } /** Dispatch change event with current value. */ diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index f246cc46bc3..32822f257cc 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -385,7 +385,7 @@ export class MdSlider implements ControlValueAccessor, OnDestroy { } ngOnDestroy() { - this._focusOriginMonitor.unmonitor(this._elementRef.nativeElement); + this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement); } _onMouseenter() {