From d83f062b8c8054c90ecc12a9cb844efe40b8927d Mon Sep 17 00:00:00 2001 From: Robert Messerle Date: Wed, 25 May 2016 10:41:16 -0700 Subject: [PATCH] Fix/403/radio without name (#478) * fix(radio): resolves change detection issues Using *ngFor to iterate over options was causing the error "Expression has changed after it was checked. Previous value: undefined." closes #403 * fix(radio): name property should be inherited from radio group --- src/components/radio/radio.spec.ts | 43 +++++++++++++++++++++++----- src/components/radio/radio.ts | 45 ++++++++++++------------------ 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/components/radio/radio.spec.ts b/src/components/radio/radio.spec.ts index 3d4333ef6256..897a3ae789e6 100644 --- a/src/components/radio/radio.spec.ts +++ b/src/components/radio/radio.spec.ts @@ -197,12 +197,11 @@ describe('MdRadio', () => { it('should deselect all of the checkboxes when the group value is cleared', () => { radioInstances[0].checked = true; - fixture.detectChanges(); expect(groupInstance.value).toBeTruthy(); groupInstance.value = null; - fixture.detectChanges(); + expect(radioInstances.every(radio => !radio.checked)).toBe(true); }); }); @@ -236,6 +235,31 @@ describe('MdRadio', () => { }); })); + it('should set individual radio names based on the group name', () => { + expect(groupInstance.name).toBeTruthy(); + for (let radio of radioInstances) { + expect(radio.name).toBe(groupInstance.name); + } + + groupInstance.name = 'new name'; + for (let radio of radioInstances) { + expect(radio.name).toBe(groupInstance.name); + } + }); + + it('should check the corresponding radio button on group value change', () => { + expect(groupInstance.value).toBeFalsy(); + for (let radio of radioInstances) { + expect(radio.checked).toBeFalsy(); + } + + groupInstance.value = 'vanilla'; + for (let radio of radioInstances) { + expect(radio.checked).toBe(groupInstance.value === radio.value); + } + expect(groupInstance.selected.value).toBe(groupInstance.value); + }); + it('should have the correct ngControl state initially and after interaction', fakeAsync(() => { // The control should start off valid, pristine, and untouched. expect(groupNgControl.valid).toBe(true); @@ -333,7 +357,7 @@ describe('MdRadio', () => { @Component({ directives: [MD_RADIO_DIRECTIVES], template: ` - + Charmander Squirtle Bulbasaur @@ -365,21 +389,26 @@ class StandaloneRadioButtons { } directives: [MD_RADIO_DIRECTIVES, FORM_DIRECTIVES], template: ` - Vanilla - Chocolate - Strawberry + + {{option.label}} + ` }) class RadioGroupWithNgModel { modelValue: string; + options = [ + {label: 'Vanilla', value: 'vanilla'}, + {label: 'Chocolate', value: 'chocolate'}, + {label: 'Strawberry', value: 'strawberry'}, + ]; } // TODO(jelbourn): remove eveything below when Angular supports faking events. /** - * Dispatches a focus change event from an element. + * Dispatches a focus change event from an element. * @param eventName Name of the event, either 'focus' or 'blur'. * @param element The element from which the event will be dispatched. */ diff --git a/src/components/radio/radio.ts b/src/components/radio/radio.ts index ec881ebdd938..fa8b496b5d12 100644 --- a/src/components/radio/radio.ts +++ b/src/components/radio/radio.ts @@ -69,7 +69,7 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { private _value: any = null; /** The HTML name attribute applied to radio buttons in this group. */ - private _name: string = null; + private _name: string = `md-radio-group-${_uniqueIdCounter++}`; /** Disables all individual radio buttons assigned to this group. */ private _disabled: boolean = false; @@ -101,14 +101,7 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { set name(value: string) { this._name = value; - this._updateChildRadioNames(); - } - - /** Propagate name attribute to radio buttons. */ - private _updateChildRadioNames(): void { - (this._radios || []).forEach(radio => { - radio.name = this._name; - }); + this._updateRadioButtonNames(); } @Input() @@ -160,10 +153,6 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { * @internal */ ngAfterContentInit() { - if (!this._name) { - this.name = `md-radio-group-${_uniqueIdCounter++}`; - } - // Mark this component as initialized in AfterContentInit because the initial value can // possibly be set by NgModel on MdRadioGroup, and it is possible that the OnInit of the // NgModel occurs *after* the OnInit of the MdRadioGroup. @@ -181,9 +170,15 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { } } + private _updateRadioButtonNames(): void { + (this._radios || []).forEach(radio => { + radio.name = this.name; + }); + } + /** Updates the `selected` radio button from the internal _value state. */ private _updateSelectedRadioFromValue(): void { - // If the value already matches the selected radio, no dothing. + // If the value already matches the selected radio, do nothing. let isAlreadySelected = this._selected != null && this._selected.value == this._value; if (this._radios != null && !isAlreadySelected) { @@ -192,8 +187,8 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { if (matchingRadio) { this.selected = matchingRadio; } else if (this.value == null) { - this.selected = null; - this._radios.forEach(radio => { radio.checked = false; }); + this.selected = null; + this._radios.forEach(radio => { radio.checked = false; }); } } } @@ -206,7 +201,7 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor { this.change.emit(event); } - /** + /** * Implemented as part of ControlValueAccessor. * @internal */ @@ -258,7 +253,7 @@ export class MdRadioButton implements OnInit { /** The unique ID for the radio button. */ @HostBinding('id') @Input() - id: string; + id: string = `md-radio-${_uniqueIdCounter++}`; /** Analog to HTML 'name' attribute used to group radios for unique selection. */ @Input() @@ -345,15 +340,11 @@ export class MdRadioButton implements OnInit { /** @internal */ ngOnInit() { - // All radio buttons must have a unique id. - if (!this.id) { - this.id = `md-radio-${_uniqueIdCounter++}`; - } - - // If the radio is inside of a radio group and it matches that group's value upon - // initialization, start off as checked. - if (this.radioGroup && this.radioGroup.value === this._value) { - this.checked = true; + if (this.radioGroup) { + // If the radio is inside a radio group, determine if it should be checked + this.checked = this.radioGroup.value === this._value; + // Copy name from parent radio group + this.name = this.radioGroup.name; } }