Skip to content

Commit

Permalink
fix(radio): only call change callback with user input (#1521)
Browse files Browse the repository at this point in the history
  • Loading branch information
kara authored Oct 25, 2016
1 parent c9ef34c commit 920c875
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 22 deletions.
44 changes: 24 additions & 20 deletions src/lib/radio/radio.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {async, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
import {NgControl, FormsModule} from '@angular/forms';
import {Component, DebugElement} from '@angular/core';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -176,12 +176,12 @@ describe('MdRadio', () => {

expect(nativeRadioInput.classList).not.toContain('md-radio-focused');

dispatchFocusChangeEvent('focus', nativeRadioInput);
dispatchEvent('focus', nativeRadioInput);
fixture.detectChanges();

expect(radioNativeElements[0].classList).toContain('md-radio-focused');

dispatchFocusChangeEvent('blur', nativeRadioInput);
dispatchEvent('blur', nativeRadioInput);
fixture.detectChanges();

expect(radioNativeElements[0].classList).not.toContain('md-radio-focused');
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('MdRadio', () => {
let groupDebugElement: DebugElement;
let groupNativeElement: HTMLElement;
let radioDebugElements: DebugElement[];
let radioNativeElements: HTMLElement[];
let innerRadios: DebugElement[];
let radioLabelElements: HTMLLabelElement[];
let groupInstance: MdRadioGroup;
let radioInstances: MdRadioButton[];
Expand All @@ -242,8 +242,8 @@ describe('MdRadio', () => {
groupNgControl = groupDebugElement.injector.get(NgControl);

radioDebugElements = fixture.debugElement.queryAll(By.directive(MdRadioButton));
radioNativeElements = radioDebugElements.map(debugEl => debugEl.nativeElement);
radioInstances = radioDebugElements.map(debugEl => debugEl.componentInstance);
innerRadios = fixture.debugElement.queryAll(By.css('input[type="radio"]'));

radioLabelElements = radioDebugElements
.map(debugEl => debugEl.query(By.css('label')).nativeElement);
Expand Down Expand Up @@ -280,16 +280,16 @@ describe('MdRadio', () => {
expect(groupNgControl.pristine).toBe(true);
expect(groupNgControl.touched).toBe(false);

// After changing the value programmatically, the control should become dirty (not pristine),
// After changing the value programmatically, the control should stay pristine
// but remain untouched.
radioInstances[1].checked = true;
fixture.detectChanges();

expect(groupNgControl.valid).toBe(true);
expect(groupNgControl.pristine).toBe(false);
expect(groupNgControl.pristine).toBe(true);
expect(groupNgControl.touched).toBe(false);

// After a user interaction occurs (such as a click), the control should remain dirty and
// After a user interaction occurs (such as a click), the control should become dirty and
// now also be touched.
radioLabelElements[2].click();
fixture.detectChanges();
Expand All @@ -299,27 +299,31 @@ describe('MdRadio', () => {
expect(groupNgControl.touched).toBe(true);
});

it('should update the ngModel value when selecting a radio button', () => {
radioInstances[1].checked = true;
it('should write to the radio button based on ngModel', fakeAsync(() => {
testComponent.modelValue = 'chocolate';
fixture.detectChanges();
tick();
fixture.detectChanges();

expect(innerRadios[1].nativeElement.checked).toBe(true);
}));

it('should update the ngModel value when selecting a radio button', () => {
dispatchEvent('change', innerRadios[1].nativeElement);
fixture.detectChanges();
expect(testComponent.modelValue).toBe('chocolate');
});

it('should update the model before firing change event', () => {
expect(testComponent.modelValue).toBeUndefined();
expect(testComponent.lastEvent).toBeUndefined();

groupInstance.value = 'chocolate';
dispatchEvent('change', innerRadios[1].nativeElement);
fixture.detectChanges();

expect(testComponent.modelValue).toBe('chocolate');
expect(testComponent.lastEvent.value).toBe('chocolate');

groupInstance.value = 'vanilla';
dispatchEvent('change', innerRadios[0].nativeElement);
fixture.detectChanges();

expect(testComponent.modelValue).toBe('vanilla');
expect(testComponent.lastEvent.value).toBe('vanilla');
});
});
Expand Down Expand Up @@ -484,14 +488,14 @@ class RadioGroupWithNgModel {
lastEvent: MdRadioChange;
}

// TODO(jelbourn): remove eveything below when Angular supports faking events.
// TODO(jelbourn): remove everything below when Angular supports faking events.

/**
* Dispatches a focus change event from an element.
* @param eventName Name of the event, either 'focus' or 'blur'.
* Dispatches an event from an element.
* @param eventName Name of the event
* @param element The element from which the event will be dispatched.
*/
function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void {
function dispatchEvent(eventName: string, element: HTMLElement): void {
let event = document.createEvent('Event');
event.initEvent(eventName, true, true);
element.dispatchEvent(event);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
private _isInitialized: boolean = false;

/** The method to be called in order to update ngModel */
private _controlValueAccessorChangeFn: (value: any) => void = (value) => {};
_controlValueAccessorChangeFn: (value: any) => void = (value) => {};

/** onTouch function registered via registerOnTouch (ControlValueAccessor). */
onTouched: () => any = () => {};
Expand Down Expand Up @@ -198,7 +198,6 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
let event = new MdRadioChange();
event.source = this._selected;
event.value = this._value;
this._controlValueAccessorChangeFn(event.value);
this.change.emit(event);
}

Expand Down Expand Up @@ -405,6 +404,7 @@ export class MdRadioButton implements OnInit {
event.stopPropagation();

this.checked = true;
this.radioGroup._controlValueAccessorChangeFn(this.value);
this._emitChangeEvent();

if (this.radioGroup) {
Expand Down

0 comments on commit 920c875

Please sign in to comment.