Skip to content

Commit

Permalink
refactor(focus-origin-monitor): rename unmonitor to stopMonitoring (#…
Browse files Browse the repository at this point in the history
…3783)

* Renames the `unmonitor` function to `stopMonitoring`.
* Removes unnecessary code that keeps track of the `FocusOriginMonitor` subscription. After calling `stopMonitoring` the observable will complete.
  • Loading branch information
devversion authored and tinayuangao committed Mar 31, 2017
1 parent a71a5af commit 2240b6c
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 50 deletions.
2 changes: 1 addition & 1 deletion src/lib/button/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`. */
Expand Down
18 changes: 4 additions & 14 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>;
let fakeFocusOriginMonitorSubject: Subject<FocusOrigin> = new Subject();
let fakeFocusOriginMonitor = {
monitor: () => fakeFocusOriginMonitorSubject.asObservable(),
unmonitor: () => {},
focusVia: (element: HTMLElement, renderer: any, focusOrigin: FocusOrigin) => {
element.focus();
fakeFocusOriginMonitorSubject.next(focusOrigin);
}
};

beforeEach(async(() => {
TestBed.configureTestingModule({
Expand All @@ -45,8 +34,7 @@ describe('MdCheckbox', () => {
CheckboxWithFormControl,
],
providers: [
{provide: ViewportRuler, useClass: FakeViewportRuler},
{provide: FocusOriginMonitor, useValue: fakeFocusOriginMonitor}
{provide: ViewportRuler, useClass: FakeViewportRuler}
]
});

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 1 addition & 6 deletions src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/lib/core/style/focus-origin-monitor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions src/lib/core/style/focus-origin-monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -296,7 +296,7 @@ export class CdkMonitorFocus implements OnDestroy {
}

ngOnDestroy() {
this._focusOriginMonitor.unmonitor(this._elementRef.nativeElement);
this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement);
}
}

Expand Down
27 changes: 9 additions & 18 deletions src/lib/radio/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<FocusOrigin>();
let fakeFocusOriginMonitor = {
monitor: () => fakeFocusOriginMonitorStream.asObservable(),
unmonitor: () => {},
focusVia: (element: HTMLElement, renderer: any, origin: FocusOrigin) => {
element.focus();
fakeFocusOriginMonitorStream.next(origin);
}
};

beforeEach(async(() => {
TestBed.configureTestingModule({
Expand All @@ -32,8 +21,7 @@ describe('MdRadio', () => {
StandaloneRadioButtons,
],
providers: [
{provide: ViewportRuler, useClass: FakeViewportRuler},
{provide: FocusOriginMonitor, useValue: fakeFocusOriginMonitor}
{provide: ViewportRuler, useClass: FakeViewportRuler}
]
});

Expand All @@ -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;
Expand All @@ -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', () => {
Expand Down Expand Up @@ -140,9 +131,7 @@ describe('MdRadio', () => {
});

it('should check a radio upon interaction with the underlying native radio button', () => {
let nativeRadioInput = <HTMLElement> radioNativeElements[0].querySelector('input');

nativeRadioInput.click();
radioInputElements[0].click();
fixture.detectChanges();

expect(radioInstances[0].checked).toBe(true);
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 1 addition & 6 deletions src/lib/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
2 changes: 1 addition & 1 deletion src/lib/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export class MdSlider implements ControlValueAccessor, OnDestroy {
}

ngOnDestroy() {
this._focusOriginMonitor.unmonitor(this._elementRef.nativeElement);
this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement);
}

_onMouseenter() {
Expand Down

0 comments on commit 2240b6c

Please sign in to comment.