From 4f7bc715be7a06ef413d71a4f45ac189c41a647f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 27 May 2016 21:06:41 +0200 Subject: [PATCH 1/3] update(): add event object for slide-toggle and checkbox. Closes #552. --- src/components/checkbox/checkbox.spec.ts | 19 +++++++---------- src/components/checkbox/checkbox.ts | 20 +++++++++++++++--- src/components/radio/radio.spec.ts | 10 ++++----- .../slide-toggle/slide-toggle.spec.ts | 21 +++++++++---------- src/components/slide-toggle/slide-toggle.ts | 19 ++++++++++++++--- 5 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/components/checkbox/checkbox.spec.ts b/src/components/checkbox/checkbox.spec.ts index 60c3c984b86d..4c6597572ccb 100644 --- a/src/components/checkbox/checkbox.spec.ts +++ b/src/components/checkbox/checkbox.spec.ts @@ -11,7 +11,7 @@ import {FORM_DIRECTIVES, NgModel, NgControl} from '@angular/common'; import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing'; import {Component, DebugElement} from '@angular/core'; import {By} from '@angular/platform-browser'; -import {MdCheckbox} from './checkbox'; +import {MdCheckbox, MdCheckboxChange} from './checkbox'; import {PromiseCompleter} from '@angular2-material/core/async/promise-completer'; @@ -273,26 +273,24 @@ describe('MdCheckbox', () => { testComponent = fixture.debugElement.componentInstance; inputElement = checkboxNativeElement.querySelector('input'); labelElement = checkboxNativeElement.querySelector('label'); - - spyOn(testComponent, 'handleChange'); }); })); it('should call the change event on first change after initialization', fakeAsync(() => { fixture.detectChanges(); - expect(testComponent.handleChange).not.toHaveBeenCalled(); + expect(testComponent.lastEvent).toBeUndefined(); checkboxInstance.checked = true; fixture.detectChanges(); tick(); - expect(testComponent.handleChange).toHaveBeenCalledTimes(1); - expect(testComponent.handleChange).toHaveBeenCalledWith(true); + expect(testComponent.lastEvent.value).toBe(true); })); it('should not emit a DOM event to the change output', async(() => { - expect(testComponent.handleChange).not.toHaveBeenCalled(); + fixture.detectChanges(); + expect(testComponent.lastEvent).toBeUndefined(); // Trigger the click on the inputElement, because the input will probably // emit a DOM event to the change output. @@ -303,8 +301,7 @@ describe('MdCheckbox', () => { // We're checking the arguments type / emitted value to be a boolean, because sometimes the // emitted value can be a DOM Event, which is not valid. // See angular/angular#4059 - expect(testComponent.handleChange).toHaveBeenCalledTimes(1); - expect(testComponent.handleChange).toHaveBeenCalledWith(true); + expect(testComponent.lastEvent.value).toBe(true); }); })); @@ -541,8 +538,8 @@ class CheckboxWithNameAttribute {} /** Simple test component with change event */ @Component({ directives: [MdCheckbox], - template: `` + template: `` }) class CheckboxWithChangeEvent { - handleChange(value: boolean): void {} + lastEvent: MdCheckboxChange; } diff --git a/src/components/checkbox/checkbox.ts b/src/components/checkbox/checkbox.ts index 65045f1c1b17..62c6d7983ec6 100644 --- a/src/components/checkbox/checkbox.ts +++ b/src/components/checkbox/checkbox.ts @@ -42,6 +42,12 @@ enum TransitionCheckState { Indeterminate } +// A simple change event emitted by the MdCheckbox component. +export class MdCheckboxChange { + source: MdCheckbox; + value: boolean; +} + /** * A material design checkbox component. Supports all of the functionality of an HTML5 checkbox, * and exposes a similar API. An MdCheckbox can be either checked, unchecked, indeterminate, or @@ -105,7 +111,7 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor { @Input() name: string = null; /** Event emitted when the checkbox's `checked` value changes. */ - @Output() change: EventEmitter = new EventEmitter(); + @Output() change: EventEmitter = new EventEmitter(); /** Called when the checkbox is blurred. Needed to properly implement ControlValueAccessor. */ onTouched: () => any = () => {}; @@ -144,7 +150,7 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor { // Only fire a change event if this isn't the first time the checked property is ever set. if (this._isInitialized) { - this.change.emit(this._checked); + this._emitChangeEvent(); } } } @@ -225,6 +231,14 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor { } } + private _emitChangeEvent() { + let event = new MdCheckboxChange(); + event.source = this; + event.value = this.checked; + + this.change.emit(event); + } + /** * Informs the component when the input has focus so that we can style accordingly * @internal @@ -258,7 +272,7 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor { onInteractionEvent(event: Event) { // We always have to stop propagation on the change event. // Otherwise the change event, from the input element, will bubble up and - // emit its event object to the `change` output. + // emit its event object to the `change` output. event.stopPropagation(); if (!this.disabled) { diff --git a/src/components/radio/radio.spec.ts b/src/components/radio/radio.spec.ts index 4515466de0fa..20644b98ee1e 100644 --- a/src/components/radio/radio.spec.ts +++ b/src/components/radio/radio.spec.ts @@ -324,20 +324,20 @@ describe('MdRadio', () => { radioInstances = radioDebugElements.map(debugEl => debugEl.componentInstance); fixture.detectChanges(); - - spyOn(testComponent, 'onChange'); }); })); it('should update the model before firing change event', fakeAsync(() => { expect(testComponent.modelValue).toBeUndefined(); + expect(testComponent.lastEvent).toBeUndefined(); groupInstance.value = 'chocolate'; fixture.detectChanges(); tick(); expect(testComponent.modelValue).toBe('chocolate'); - expect(testComponent.onChange).toHaveBeenCalledWith('chocolate'); + + expect(testComponent.lastEvent.value).toBe('chocolate'); })); }); @@ -432,7 +432,7 @@ class StandaloneRadioButtons { } @Component({ directives: [MD_RADIO_DIRECTIVES, FORM_DIRECTIVES], template: ` - + {{option.label}} @@ -446,7 +446,7 @@ class RadioGroupWithNgModel { {label: 'Chocolate', value: 'chocolate'}, {label: 'Strawberry', value: 'strawberry'}, ]; - onChange(value: MdRadioChange) {} + lastEvent: MdRadioChange; } // TODO(jelbourn): remove eveything below when Angular supports faking events. diff --git a/src/components/slide-toggle/slide-toggle.spec.ts b/src/components/slide-toggle/slide-toggle.spec.ts index eabae926fd81..ff43b2106dcc 100644 --- a/src/components/slide-toggle/slide-toggle.spec.ts +++ b/src/components/slide-toggle/slide-toggle.spec.ts @@ -10,7 +10,7 @@ import { import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing'; import {By} from '@angular/platform-browser'; import {Component} from '@angular/core'; -import {MdSlideToggle} from './slide-toggle'; +import {MdSlideToggle, MdSlideToggleChange} from './slide-toggle'; import {NgControl} from '@angular/common'; describe('MdSlideToggle', () => { @@ -162,21 +162,21 @@ describe('MdSlideToggle', () => { expect(slideToggleElement.classList).not.toContain('ng-dirty'); }); - it('should emit the new values properly', fakeAsync(() => { - spyOn(testComponent, 'onValueChange'); - + it('should emit the new values properly', async(() => { labelElement.click(); fixture.detectChanges(); fixture.whenStable().then(() => { - expect(testComponent.onValueChange).toHaveBeenCalledTimes(1); - expect(testComponent.onValueChange).toHaveBeenCalledWith(true); + // We're checking the arguments type / emitted value to be a boolean, because sometimes the + // emitted value can be a DOM Event, which is not valid. + // See angular/angular#4059 + expect(testComponent.lastEvent.value).toBe(true); }); })); it('should support subscription on the change observable', () => { - slideToggle.change.subscribe(value => { - expect(value).toBe(true); + slideToggle.change.subscribe((event: MdSlideToggleChange) => { + expect(event.value).toBe(true); }); slideToggle.toggle(); @@ -269,7 +269,7 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void + [ariaLabelledby]="slideLabelledBy" (change)="lastEvent = $event"> Test Slide Toggle `, @@ -284,6 +284,5 @@ class SlideToggleTestApp { slideName: string; slideLabel: string; slideLabelledBy: string; - - onValueChange(value: boolean): void {}; + lastEvent: MdSlideToggleChange; } diff --git a/src/components/slide-toggle/slide-toggle.ts b/src/components/slide-toggle/slide-toggle.ts index 17927dc03f10..2f10ec41f5d1 100644 --- a/src/components/slide-toggle/slide-toggle.ts +++ b/src/components/slide-toggle/slide-toggle.ts @@ -21,6 +21,12 @@ export const MD_SLIDE_TOGGLE_VALUE_ACCESSOR: any = { multi: true }; +// A simple change event emitted by the MdSlideToggle component. +export class MdSlideToggleChange { + source: MdSlideToggle; + value: boolean; +} + // Increasing integer for generating unique ids for slide-toggle components. let nextId = 0; @@ -59,8 +65,8 @@ export class MdSlideToggle implements ControlValueAccessor { @Input() ariaLabel: string = null; @Input() ariaLabelledby: string = null; - @Output('change') private _change: EventEmitter = new EventEmitter(); - change: Observable = this._change.asObservable(); + @Output('change') private _change: EventEmitter = new EventEmitter(); + change: Observable = this._change.asObservable(); // Returns the unique id for the visual hidden input. getInputId = () => `${this.id || this._uniqueId}-input`; @@ -144,7 +150,7 @@ export class MdSlideToggle implements ControlValueAccessor { if (this.checked !== !!value) { this._checked = value; this.onChange(this._checked); - this._change.emit(this._checked); + this._emitChangeEvent(); } } @@ -173,6 +179,13 @@ export class MdSlideToggle implements ControlValueAccessor { } } + private _emitChangeEvent() { + let event = new MdSlideToggleChange(); + event.source = this; + event.value = this.checked; + this._change.emit(event); + } + } export const MD_SLIDE_TOGGLE_DIRECTIVES = [MdSlideToggle]; From a249c39e272fb071a5bdce477b9f9baa37f86374 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 27 May 2016 21:21:23 +0200 Subject: [PATCH 2/3] Fix Linters... again :P --- src/components/radio/radio.spec.ts | 1 - src/components/slide-toggle/slide-toggle.spec.ts | 3 +-- src/components/slide-toggle/slide-toggle.ts | 4 +++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/radio/radio.spec.ts b/src/components/radio/radio.spec.ts index 20644b98ee1e..6f5a8c1a6de4 100644 --- a/src/components/radio/radio.spec.ts +++ b/src/components/radio/radio.spec.ts @@ -336,7 +336,6 @@ describe('MdRadio', () => { tick(); expect(testComponent.modelValue).toBe('chocolate'); - expect(testComponent.lastEvent.value).toBe('chocolate'); })); }); diff --git a/src/components/slide-toggle/slide-toggle.spec.ts b/src/components/slide-toggle/slide-toggle.spec.ts index ff43b2106dcc..269013c838d2 100644 --- a/src/components/slide-toggle/slide-toggle.spec.ts +++ b/src/components/slide-toggle/slide-toggle.spec.ts @@ -4,8 +4,7 @@ import { expect, beforeEach, inject, - async, - fakeAsync, + async } from '@angular/core/testing'; import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing'; import {By} from '@angular/platform-browser'; diff --git a/src/components/slide-toggle/slide-toggle.ts b/src/components/slide-toggle/slide-toggle.ts index 2f10ec41f5d1..2997e06e223f 100644 --- a/src/components/slide-toggle/slide-toggle.ts +++ b/src/components/slide-toggle/slide-toggle.ts @@ -65,7 +65,9 @@ export class MdSlideToggle implements ControlValueAccessor { @Input() ariaLabel: string = null; @Input() ariaLabelledby: string = null; - @Output('change') private _change: EventEmitter = new EventEmitter(); + @Output('change') + private _change: EventEmitter = new EventEmitter(); + change: Observable = this._change.asObservable(); // Returns the unique id for the visual hidden input. From 9a371c92c9b0b11a68644e5eb54f5151db90ebb6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 27 May 2016 22:56:28 +0200 Subject: [PATCH 3/3] update(): Address comments from @jelborun --- src/components/checkbox/checkbox.spec.ts | 4 ++-- src/components/checkbox/checkbox.ts | 4 ++-- src/components/slide-toggle/slide-toggle.spec.ts | 4 ++-- src/components/slide-toggle/slide-toggle.ts | 8 +++----- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/checkbox/checkbox.spec.ts b/src/components/checkbox/checkbox.spec.ts index 4c6597572ccb..0364f34f8698 100644 --- a/src/components/checkbox/checkbox.spec.ts +++ b/src/components/checkbox/checkbox.spec.ts @@ -285,7 +285,7 @@ describe('MdCheckbox', () => { tick(); - expect(testComponent.lastEvent.value).toBe(true); + expect(testComponent.lastEvent.checked).toBe(true); })); it('should not emit a DOM event to the change output', async(() => { @@ -301,7 +301,7 @@ describe('MdCheckbox', () => { // We're checking the arguments type / emitted value to be a boolean, because sometimes the // emitted value can be a DOM Event, which is not valid. // See angular/angular#4059 - expect(testComponent.lastEvent.value).toBe(true); + expect(testComponent.lastEvent.checked).toBe(true); }); })); diff --git a/src/components/checkbox/checkbox.ts b/src/components/checkbox/checkbox.ts index 62c6d7983ec6..10b7e5ff0e74 100644 --- a/src/components/checkbox/checkbox.ts +++ b/src/components/checkbox/checkbox.ts @@ -45,7 +45,7 @@ enum TransitionCheckState { // A simple change event emitted by the MdCheckbox component. export class MdCheckboxChange { source: MdCheckbox; - value: boolean; + checked: boolean; } /** @@ -234,7 +234,7 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor { private _emitChangeEvent() { let event = new MdCheckboxChange(); event.source = this; - event.value = this.checked; + event.checked = this.checked; this.change.emit(event); } diff --git a/src/components/slide-toggle/slide-toggle.spec.ts b/src/components/slide-toggle/slide-toggle.spec.ts index 269013c838d2..cd5bf113812a 100644 --- a/src/components/slide-toggle/slide-toggle.spec.ts +++ b/src/components/slide-toggle/slide-toggle.spec.ts @@ -169,13 +169,13 @@ describe('MdSlideToggle', () => { // We're checking the arguments type / emitted value to be a boolean, because sometimes the // emitted value can be a DOM Event, which is not valid. // See angular/angular#4059 - expect(testComponent.lastEvent.value).toBe(true); + expect(testComponent.lastEvent.checked).toBe(true); }); })); it('should support subscription on the change observable', () => { slideToggle.change.subscribe((event: MdSlideToggleChange) => { - expect(event.value).toBe(true); + expect(event.checked).toBe(true); }); slideToggle.toggle(); diff --git a/src/components/slide-toggle/slide-toggle.ts b/src/components/slide-toggle/slide-toggle.ts index 2997e06e223f..ade15efcf82c 100644 --- a/src/components/slide-toggle/slide-toggle.ts +++ b/src/components/slide-toggle/slide-toggle.ts @@ -24,7 +24,7 @@ export const MD_SLIDE_TOGGLE_VALUE_ACCESSOR: any = { // A simple change event emitted by the MdSlideToggle component. export class MdSlideToggleChange { source: MdSlideToggle; - value: boolean; + checked: boolean; } // Increasing integer for generating unique ids for slide-toggle components. @@ -65,10 +65,8 @@ export class MdSlideToggle implements ControlValueAccessor { @Input() ariaLabel: string = null; @Input() ariaLabelledby: string = null; - @Output('change') private _change: EventEmitter = new EventEmitter(); - - change: Observable = this._change.asObservable(); + @Output() change: Observable = this._change.asObservable(); // Returns the unique id for the visual hidden input. getInputId = () => `${this.id || this._uniqueId}-input`; @@ -184,7 +182,7 @@ export class MdSlideToggle implements ControlValueAccessor { private _emitChangeEvent() { let event = new MdSlideToggleChange(); event.source = this; - event.value = this.checked; + event.checked = this.checked; this._change.emit(event); }