Skip to content

Commit a3b2811

Browse files
committed
fix(radio): set tabindex based on selected state
Currently all radio buttons inside a radio group have a `tabindex` of 0, unless they're disabled, and we leave it up to the browser to focus the proper button based on its selected state when the user is tabbing. This works for the most part, but it breaks down with something like our focus trap which determines which element to focus based on its `tabindex` since all buttons have the same `tabindex`. These changes fix the issue by setting a `tabindex` of 0 only on the selected radio button. Fixes #17876.
1 parent 09dc459 commit a3b2811

File tree

4 files changed

+66
-1
lines changed

4 files changed

+66
-1
lines changed

src/material/radio/radio.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
[id]="inputId"
2020
[checked]="checked"
2121
[disabled]="disabled"
22-
[tabIndex]="tabIndex"
22+
[tabIndex]="_getInputTabindex()"
2323
[attr.name]="name"
2424
[attr.value]="value"
2525
[required]="required"

src/material/radio/radio.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('MatRadio', () => {
2323
TranscludingWrapper,
2424
RadioButtonWithPredefinedTabindex,
2525
RadioButtonWithPredefinedAriaAttributes,
26+
RadiosInsidePreCheckedRadioGroup,
2627
]
2728
});
2829

@@ -387,6 +388,41 @@ describe('MatRadio', () => {
387388
expect(radioNativeElements[2].classList).toContain('mat-warn');
388389
});
389390

391+
it('should set the input tabindex based on the selected radio button', () => {
392+
const getTabIndexes = () => {
393+
return radioInputElements.map(element => parseInt(element.getAttribute('tabindex') || ''));
394+
};
395+
396+
expect(getTabIndexes()).toEqual([0, 0, 0]);
397+
398+
radioLabelElements[0].click();
399+
fixture.detectChanges();
400+
401+
expect(getTabIndexes()).toEqual([0, -1, -1]);
402+
403+
radioLabelElements[1].click();
404+
fixture.detectChanges();
405+
406+
expect(getTabIndexes()).toEqual([-1, 0, -1]);
407+
408+
radioLabelElements[2].click();
409+
fixture.detectChanges();
410+
411+
expect(getTabIndexes()).toEqual([-1, -1, 0]);
412+
});
413+
414+
it('should set the input tabindex correctly with a pre-checked radio button', () => {
415+
const precheckedFixture = TestBed.createComponent(RadiosInsidePreCheckedRadioGroup);
416+
precheckedFixture.detectChanges();
417+
418+
const radios: NodeListOf<HTMLElement> =
419+
precheckedFixture.nativeElement.querySelectorAll('mat-radio-button input');
420+
421+
expect(Array.from(radios).map(radio => {
422+
return radio.getAttribute('tabindex');
423+
})).toEqual(['-1', '-1', '0']);
424+
});
425+
390426
});
391427

392428
describe('group with ngModel', () => {
@@ -884,6 +920,18 @@ class RadiosInsideRadioGroup {
884920
color: string | null;
885921
}
886922

923+
@Component({
924+
template: `
925+
<mat-radio-group name="test-name">
926+
<mat-radio-button value="fire">Charmander</mat-radio-button>
927+
<mat-radio-button value="water">Squirtle</mat-radio-button>
928+
<mat-radio-button value="leaf" checked>Bulbasaur</mat-radio-button>
929+
</mat-radio-group>
930+
`
931+
})
932+
class RadiosInsidePreCheckedRadioGroup {
933+
}
934+
887935

888936
@Component({
889937
template: `

src/material/radio/radio.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ export class MatRadioGroup implements AfterContentInit, ControlValueAccessor {
192192
this._selected = selected;
193193
this.value = selected ? selected.value : null;
194194
this._checkSelectedRadioButton();
195+
this._markRadiosForCheck();
195196
}
196197

197198
/** Whether the radio group is disabled */
@@ -597,6 +598,21 @@ export class MatRadioButton extends _MatRadioButtonMixinBase
597598
}
598599
}
599600

601+
/** Gets the tabindex for the underlying input element. */
602+
_getInputTabindex(): number {
603+
const group = this.radioGroup;
604+
605+
// Implement a roving tabindex if the button is inside a group. For most cases this isn't
606+
// necessary, because the browser handles the tab order for inputs inside a group automatically,
607+
// but we need an explicitly higher tabindex for the selected button in order for things like
608+
// the focus trap to pick it up correctly.
609+
if (!group || !group.selected || this.disabled) {
610+
return this.tabIndex;
611+
}
612+
613+
return group.selected === this ? this.tabIndex : -1;
614+
}
615+
600616
static ngAcceptInputType_checked: BooleanInput;
601617
static ngAcceptInputType_disabled: BooleanInput;
602618
static ngAcceptInputType_required: BooleanInput;

tools/public_api_guard/material/radio.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export declare class MatRadioButton extends _MatRadioButtonMixinBase implements
2222
required: boolean;
2323
value: any;
2424
constructor(radioGroup: MatRadioGroup, elementRef: ElementRef, _changeDetector: ChangeDetectorRef, _focusMonitor: FocusMonitor, _radioDispatcher: UniqueSelectionDispatcher, _animationMode?: string | undefined, _providerOverride?: MatRadioDefaultOptions | undefined);
25+
_getInputTabindex(): number;
2526
_isRippleDisabled(): boolean;
2627
_markForCheck(): void;
2728
_onInputChange(event: Event): void;

0 commit comments

Comments
 (0)