Skip to content

Commit

Permalink
fix(radio): only emit change event on user interaction (#1680)
Browse files Browse the repository at this point in the history
  • Loading branch information
tinayuangao authored and jelbourn committed Nov 3, 2016
1 parent 7336b90 commit 0d552f5
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
25 changes: 14 additions & 11 deletions src/lib/radio/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,22 @@ describe('MdRadio', () => {
expect(spies[1]).toHaveBeenCalledTimes(1);
});

it('should emit a change event from the radio group', () => {
it(`should not emit a change event from the radio group when change group value
programmatically`, () => {
expect(groupInstance.value).toBeFalsy();

let changeSpy = jasmine.createSpy('radio-group change listener');
groupInstance.change.subscribe(changeSpy);

groupInstance.value = 'fire';
radioLabelElements[0].click();
fixture.detectChanges();

expect(changeSpy).toHaveBeenCalled();
expect(changeSpy).toHaveBeenCalledTimes(1);

groupInstance.value = 'water';
fixture.detectChanges();

expect(changeSpy).toHaveBeenCalledTimes(2);
expect(changeSpy).toHaveBeenCalledTimes(1);
});

// TODO(jelbourn): test this in an e2e test with *real* focus, rather than faking
Expand Down Expand Up @@ -234,42 +235,44 @@ describe('MdRadio', () => {
}
}));

it('should update the group\'s selected radio to null when unchecking that radio '
+ 'programmatically', () => {
it(`should update the group's selected radio to null when unchecking that radio
programmatically`, () => {
let changeSpy = jasmine.createSpy('radio-group change listener');
groupInstance.change.subscribe(changeSpy);
radioInstances[0].checked = true;

fixture.detectChanges();

expect(changeSpy).toHaveBeenCalled();
expect(changeSpy).not.toHaveBeenCalled();
expect(groupInstance.value).toBeTruthy();

radioInstances[0].checked = false;

fixture.detectChanges();

expect(changeSpy).toHaveBeenCalledTimes(2);
expect(changeSpy).not.toHaveBeenCalled();
expect(groupInstance.value).toBeFalsy();
expect(radioInstances.every(radio => !radio.checked)).toBe(true);
expect(groupInstance.selected).toBeNull();
});

it('should fire a change event from the group whenever a radio checked state changes', () => {
it('should not fire a change event from the group when a radio checked state changes', () => {
let changeSpy = jasmine.createSpy('radio-group change listener');
groupInstance.change.subscribe(changeSpy);
radioInstances[0].checked = true;

fixture.detectChanges();

expect(changeSpy).toHaveBeenCalled();
expect(changeSpy).not.toHaveBeenCalled();
expect(groupInstance.value).toBeTruthy();
expect(groupInstance.value).toBe('fire');

radioInstances[1].checked = true;

fixture.detectChanges();

expect(changeSpy).toHaveBeenCalledTimes(2);
expect(groupInstance.value).toBe('water');
expect(changeSpy).not.toHaveBeenCalled();
});
});

Expand Down
21 changes: 11 additions & 10 deletions src/lib/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,6 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
this._value = newValue;

this._updateSelectedRadioFromValue();

// Only fire a change event if this isn't the first time the value is ever set.
if (this._isInitialized) {
this._emitChangeEvent();
}
}
}

Expand Down Expand Up @@ -197,11 +192,13 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
}

/** Dispatch change event with current selection and group value. */
private _emitChangeEvent(): void {
let event = new MdRadioChange();
event.source = this._selected;
event.value = this._value;
this.change.emit(event);
_emitChangeEvent(): void {
if (this._isInitialized) {
let event = new MdRadioChange();
event.source = this._selected;
event.value = this._value;
this.change.emit(event);
}
}

/**
Expand Down Expand Up @@ -418,12 +415,16 @@ export class MdRadioButton implements OnInit {
// emit its event object to the `change` output.
event.stopPropagation();

let groupValueChanged = this.radioGroup && this.value != this.radioGroup.value;
this.checked = true;
this._emitChangeEvent();

if (this.radioGroup) {
this.radioGroup._controlValueAccessorChangeFn(this.value);
this.radioGroup._touch();
if (groupValueChanged) {
this.radioGroup._emitChangeEvent();
}
}
}

Expand Down

0 comments on commit 0d552f5

Please sign in to comment.