Skip to content

Commit

Permalink
fix(buttons): set name on radio inputs to fix keyboard navigation
Browse files Browse the repository at this point in the history
fixes #1704

Closes #1706
  • Loading branch information
jnizet authored and pkozlowski-opensource committed Jul 27, 2017
1 parent ae4e3e9 commit 3bfd82d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 1 deletion.
63 changes: 63 additions & 0 deletions src/buttons/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ function expectRadios(element: HTMLElement, states: number[]) {
}
}

function expectNameOnAllInputs(element: HTMLElement, name: string) {
const inputs = element.querySelectorAll('input');
for (let i = 0; i < inputs.length; i++) {
expect(inputs[i].getAttribute('name')).toBe(name);
}
}

function getGroupElement(nativeEl: HTMLElement): HTMLDivElement {
return <HTMLDivElement>nativeEl.querySelector('div[ngbRadioGroup]');
}
Expand Down Expand Up @@ -470,6 +477,62 @@ describe('ngbRadioGroup', () => {
expect(inputDebugEls[1].nativeElement.parentNode).toHaveCssClass('focus');
});

it('should generate input names automatically if no name specified anywhere', () => {
const fixture = createTestComponent(`
<div [(ngModel)]="model" ngbRadioGroup>
<label ngbButtonLabel>
<input ngbButton type="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label ngbButtonLabel>
<input ngbButton type="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>
`);
fixture.detectChanges();

const inputs = fixture.nativeElement.querySelectorAll('input');
const distinctNames = new Set();
for (let i = 0; i < inputs.length; i++) {
distinctNames.add(inputs[i].getAttribute('name'));
}
expect(distinctNames.size).toBe(1);
expect(distinctNames.values().next().value).toMatch(/ngb-radio-\d+/);
});

it('should set input names from group name if inputs don\'t have a name', () => {
const fixture = createTestComponent(`
<div [(ngModel)]="model" ngbRadioGroup name="foo">
<label ngbButtonLabel>
<input ngbButton type="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label ngbButtonLabel>
<input ngbButton type="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>
`);
fixture.detectChanges();

const inputs = fixture.nativeElement.querySelectorAll('input');
expectNameOnAllInputs(fixture.nativeElement, 'foo');
});

it('should honor the input names if specified', () => {
const fixture = createTestComponent(`
<div [(ngModel)]="model" ngbRadioGroup name="foo">
<label ngbButtonLabel>
<input ngbButton name="bar" type="radio" [value]="values[0]"/> {{ values[0] }}
</label>
<label ngbButtonLabel>
<input ngbButton [name]="'bar'" type="radio" [value]="values[1]"/> {{ values[1] }}
</label>
</div>
`);
fixture.detectChanges();

const inputs = fixture.nativeElement.querySelectorAll('input');
expectNameOnAllInputs(fixture.nativeElement, 'bar');
});

describe('accessibility', () => {
it('should have "group" role', () => {
const fixture = TestBed.createComponent(TestComponent);
Expand Down
19 changes: 18 additions & 1 deletion src/buttons/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const NGB_RADIO_VALUE_ACCESSOR = {
multi: true
};

let nextId = 0;

/**
* Easily create Bootstrap-style radio buttons. A value of a selected button is bound to a variable
* specified via ngModel.
Expand All @@ -26,6 +28,12 @@ export class NgbRadioGroup implements ControlValueAccessor {
get disabled() { return this._disabled; }
set disabled(isDisabled: boolean) { this.setDisabledState(isDisabled); }

/**
* The name of the group. Unless enclosed inputs specify a name, this name is used as the name of the
* enclosed inputs. If not specified, a name is generated automatically.
*/
@Input() name = `ngb-radio-${nextId++}`;

onChange = (_: any) => {};
onTouched = () => {};

Expand Down Expand Up @@ -67,6 +75,7 @@ export class NgbRadioGroup implements ControlValueAccessor {
host: {
'[checked]': 'checked',
'[disabled]': 'disabled',
'[name]': 'nameAttr',
'(change)': 'onChange()',
'(focus)': 'focused = true',
'(blur)': 'focused = false'
Expand All @@ -77,9 +86,15 @@ export class NgbRadio implements OnDestroy {
private _disabled: boolean;
private _value: any = null;

/**
* The name of the input. All inputs of a group should have the same name. If not specified,
* the name of the enclosing group is used.
*/
@Input() name: string;

/**
* You can specify model value of a given radio by binding to the value property.
*/
*/
@Input('value')
set value(value: any) {
this._value = value;
Expand Down Expand Up @@ -109,6 +124,8 @@ export class NgbRadio implements OnDestroy {

get value() { return this._value; }

get nameAttr() { return this.name || this._group.name; }

constructor(
private _group: NgbRadioGroup, private _label: NgbButtonLabel, private _renderer: Renderer2,
private _element: ElementRef) {
Expand Down

0 comments on commit 3bfd82d

Please sign in to comment.