Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(radio): model should update before change event is fired #456

Merged
merged 1 commit into from
May 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions src/components/radio/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,50 @@ describe('MdRadio', () => {
}));
});

describe('group with ngModel and change event', () => {
Copy link
Member

@jelbourn jelbourn May 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the existing "group with ngModel" block? You can just add a change event to the existing test component without affecting the other tests.

let fixture: ComponentFixture<RadioGroupWithNgModel>;
let groupDebugElement: DebugElement;
let groupNativeElement: HTMLElement;
let radioDebugElements: DebugElement[];
let radioNativeElements: HTMLElement[];
let groupInstance: MdRadioGroup;
let radioInstances: MdRadioButton[];
let testComponent: RadioGroupWithNgModel;
let groupNgControl: NgControl;

beforeEach(async(() => {
builder.createAsync(RadioGroupWithNgModel).then(f => {
fixture = f;

testComponent = fixture.componentInstance;

groupDebugElement = fixture.debugElement.query(By.directive(MdRadioGroup));
groupNativeElement = groupDebugElement.nativeElement;
groupInstance = groupDebugElement.injector.get(MdRadioGroup);
groupNgControl = groupDebugElement.injector.get(NgControl);

radioDebugElements = fixture.debugElement.queryAll(By.directive(MdRadioButton));
radioNativeElements = radioDebugElements.map(debugEl => debugEl.nativeElement);
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();

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

tick();
expect(testComponent.modelValue).toBe('chocolate');
expect(testComponent.onChange).toHaveBeenCalledWith('chocolate');
}));
});

describe('as standalone', () => {
let fixture: ComponentFixture<StandaloneRadioButtons>;
let radioDebugElements: DebugElement[];
Expand Down Expand Up @@ -388,7 +432,7 @@ class StandaloneRadioButtons { }
@Component({
directives: [MD_RADIO_DIRECTIVES, FORM_DIRECTIVES],
template: `
<md-radio-group [(ngModel)]="modelValue">
<md-radio-group [(ngModel)]="modelValue" (change)="onChange(modelValue)">
<md-radio-button *ngFor="let option of options" [value]="option.value">
{{option.label}}
</md-radio-button>
Expand All @@ -402,11 +446,11 @@ class RadioGroupWithNgModel {
{label: 'Chocolate', value: 'chocolate'},
{label: 'Strawberry', value: 'strawberry'},
];
onChange(value: string) {}
}

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


/**
* Dispatches a focus change event from an element.
* @param eventName Name of the event, either 'focus' or 'blur'.
Expand Down
15 changes: 5 additions & 10 deletions src/components/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
/** Whether the `value` has been set to its initial value. */
private _isInitialized: boolean = false;

/** Change event subscription set up by registerOnChange (ControlValueAccessor). */
private _changeSubscription: {unsubscribe: () => any} = null;
/** The method to be called in order to update ngModel */
private _controlValueAccessorChangeFn: (value: any) => void = (value) => {};

/** onTouch function registered via registerOnTouch (ControlValueAccessor). */
onTouched: () => any = () => {};
Expand Down Expand Up @@ -198,6 +198,7 @@ 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 All @@ -213,14 +214,8 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
* Implemented as part of ControlValueAccessor.
* @internal
*/
registerOnChange(fn: any) {
if (this._changeSubscription) {
this._changeSubscription.unsubscribe();
}

this._changeSubscription = this.change.subscribe((changeEvent: MdRadioChange) => {
fn(changeEvent.value);
});
registerOnChange(fn: (value: any) => void) {
this._controlValueAccessorChangeFn = fn;
}

/**
Expand Down