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/403/radio without name #478

Merged
merged 2 commits into from
May 25, 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
43 changes: 36 additions & 7 deletions src/components/radio/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -333,7 +357,7 @@ describe('MdRadio', () => {
@Component({
directives: [MD_RADIO_DIRECTIVES],
template: `
<md-radio-group [disabled]="isGroupDisabled" [value]="groupValue">
<md-radio-group [disabled]="isGroupDisabled" [value]="groupValue" name="test-name">
<md-radio-button value="fire">Charmander</md-radio-button>
<md-radio-button value="water">Squirtle</md-radio-button>
<md-radio-button value="leaf">Bulbasaur</md-radio-button>
Expand Down Expand Up @@ -365,21 +389,26 @@ class StandaloneRadioButtons { }
directives: [MD_RADIO_DIRECTIVES, FORM_DIRECTIVES],
template: `
<md-radio-group [(ngModel)]="modelValue">
<md-radio-button value="vanilla">Vanilla</md-radio-button>
<md-radio-button value="chocolate">Chocolate</md-radio-button>
<md-radio-button value="strawberry">Strawberry</md-radio-button>
<md-radio-button *ngFor="let option of options" [value]="option.value">
{{option.label}}
</md-radio-button>
</md-radio-group>
`
})
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.
*/
Expand Down
45 changes: 18 additions & 27 deletions src/components/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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; });
}
}
}
Expand All @@ -206,7 +201,7 @@ export class MdRadioGroup implements AfterContentInit, ControlValueAccessor {
this.change.emit(event);
}

/**
/**
* Implemented as part of ControlValueAccessor.
* @internal
*/
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
}
}

Expand Down