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

closes #448

Author: robertmesserle

src/components/radio/radio.spec.ts

@@ -297,6 +297,50 @@ describe('MdRadio', () => {
     }));
   });
 
+  describe('group with ngModel and change event', () => {
+    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[];
@@ -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>
@@ -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'.

src/components/radio/radio.ts

@@ -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 = () => {};
@@ -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);
   }
 
@@ -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;
   }
 
   /**