Skip to content

Commit 8fc4a2a

Browse files
committed
fix(material/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 8424209 commit 8fc4a2a

File tree

6 files changed

+116
-2
lines changed

6 files changed

+116
-2
lines changed

src/material-experimental/mdc-radio/radio.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
[id]="inputId"
88
[checked]="checked"
99
[disabled]="disabled"
10-
[tabIndex]="tabIndex"
10+
[tabIndex]="_getInputTabindex()"
1111
[attr.name]="name"
1212
[attr.value]="value"
1313
[required]="required"

src/material-experimental/mdc-radio/radio.spec.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('MDC-based MatRadio', () => {
2626
TranscludingWrapper,
2727
RadioButtonWithPredefinedTabindex,
2828
RadioButtonWithPredefinedAriaAttributes,
29+
RadiosInsidePreCheckedRadioGroup,
2930
]
3031
});
3132

@@ -412,6 +413,41 @@ describe('MDC-based MatRadio', () => {
412413
.every(element => element.classList.contains('mat-mdc-focus-indicator'))).toBe(true);
413414
});
414415

416+
it('should set the input tabindex based on the selected radio button', () => {
417+
const getTabIndexes = () => {
418+
return radioInputElements.map(element => parseInt(element.getAttribute('tabindex') || ''));
419+
};
420+
421+
expect(getTabIndexes()).toEqual([0, 0, 0]);
422+
423+
radioLabelElements[0].click();
424+
fixture.detectChanges();
425+
426+
expect(getTabIndexes()).toEqual([0, -1, -1]);
427+
428+
radioLabelElements[1].click();
429+
fixture.detectChanges();
430+
431+
expect(getTabIndexes()).toEqual([-1, 0, -1]);
432+
433+
radioLabelElements[2].click();
434+
fixture.detectChanges();
435+
436+
expect(getTabIndexes()).toEqual([-1, -1, 0]);
437+
});
438+
439+
it('should set the input tabindex correctly with a pre-checked radio button', () => {
440+
const precheckedFixture = TestBed.createComponent(RadiosInsidePreCheckedRadioGroup);
441+
precheckedFixture.detectChanges();
442+
443+
const radios: NodeListOf<HTMLElement> =
444+
precheckedFixture.nativeElement.querySelectorAll('mat-radio-button input');
445+
446+
expect(Array.from(radios).map(radio => {
447+
return radio.getAttribute('tabindex');
448+
})).toEqual(['-1', '-1', '0']);
449+
});
450+
415451
});
416452

417453
describe('group with ngModel', () => {
@@ -934,6 +970,19 @@ class RadiosInsideRadioGroup {
934970
}
935971

936972

973+
@Component({
974+
template: `
975+
<mat-radio-group name="test-name">
976+
<mat-radio-button value="fire">Charmander</mat-radio-button>
977+
<mat-radio-button value="water">Squirtle</mat-radio-button>
978+
<mat-radio-button value="leaf" checked>Bulbasaur</mat-radio-button>
979+
</mat-radio-group>
980+
`
981+
})
982+
class RadiosInsidePreCheckedRadioGroup {
983+
}
984+
985+
937986
@Component({
938987
template: `
939988
<mat-radio-button name="season" value="spring">Spring</mat-radio-button>

src/material/radio/radio.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
[id]="inputId"
1010
[checked]="checked"
1111
[disabled]="disabled"
12-
[tabIndex]="tabIndex"
12+
[tabIndex]="_getInputTabindex()"
1313
[attr.name]="name"
1414
[attr.value]="value"
1515
[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

@@ -407,6 +408,41 @@ describe('MatRadio', () => {
407408
.every(element => element.classList.contains('mat-focus-indicator'))).toBe(true);
408409
});
409410

411+
it('should set the input tabindex based on the selected radio button', () => {
412+
const getTabIndexes = () => {
413+
return radioInputElements.map(element => parseInt(element.getAttribute('tabindex') || ''));
414+
};
415+
416+
expect(getTabIndexes()).toEqual([0, 0, 0]);
417+
418+
radioLabelElements[0].click();
419+
fixture.detectChanges();
420+
421+
expect(getTabIndexes()).toEqual([0, -1, -1]);
422+
423+
radioLabelElements[1].click();
424+
fixture.detectChanges();
425+
426+
expect(getTabIndexes()).toEqual([-1, 0, -1]);
427+
428+
radioLabelElements[2].click();
429+
fixture.detectChanges();
430+
431+
expect(getTabIndexes()).toEqual([-1, -1, 0]);
432+
});
433+
434+
it('should set the input tabindex correctly with a pre-checked radio button', () => {
435+
const precheckedFixture = TestBed.createComponent(RadiosInsidePreCheckedRadioGroup);
436+
precheckedFixture.detectChanges();
437+
438+
const radios: NodeListOf<HTMLElement> =
439+
precheckedFixture.nativeElement.querySelectorAll('mat-radio-button input');
440+
441+
expect(Array.from(radios).map(radio => {
442+
return radio.getAttribute('tabindex');
443+
})).toEqual(['-1', '-1', '0']);
444+
});
445+
410446
});
411447

412448
describe('group with ngModel', () => {
@@ -920,6 +956,18 @@ class RadiosInsideRadioGroup {
920956
color: string | null;
921957
}
922958

959+
@Component({
960+
template: `
961+
<mat-radio-group name="test-name">
962+
<mat-radio-button value="fire">Charmander</mat-radio-button>
963+
<mat-radio-button value="water">Squirtle</mat-radio-button>
964+
<mat-radio-button value="leaf" checked>Bulbasaur</mat-radio-button>
965+
</mat-radio-group>
966+
`
967+
})
968+
class RadiosInsidePreCheckedRadioGroup {
969+
}
970+
923971

924972
@Component({
925973
template: `

src/material/radio/radio.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase> implemen
197197
this._selected = selected;
198198
this.value = selected ? selected.value : null;
199199
this._checkSelectedRadioButton();
200+
this._markRadiosForCheck();
200201
}
201202

202203
/** Whether the radio group is disabled */
@@ -614,6 +615,21 @@ export abstract class _MatRadioButtonBase extends _MatRadioButtonMixinBase imple
614615
}
615616
}
616617

618+
/** Gets the tabindex for the underlying input element. */
619+
_getInputTabindex(): number {
620+
const group = this.radioGroup;
621+
622+
// Implement a roving tabindex if the button is inside a group. For most cases this isn't
623+
// necessary, because the browser handles the tab order for inputs inside a group automatically,
624+
// but we need an explicitly higher tabindex for the selected button in order for things like
625+
// the focus trap to pick it up correctly.
626+
if (!group || !group.selected || this.disabled) {
627+
return this.tabIndex;
628+
}
629+
630+
return group.selected === this ? this.tabIndex : -1;
631+
}
632+
617633
static ngAcceptInputType_checked: BooleanInput;
618634
static ngAcceptInputType_disabled: BooleanInput;
619635
static ngAcceptInputType_required: BooleanInput;

tools/public_api_guard/material/radio.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export abstract class _MatRadioButtonBase extends _MatRadioButtonMixinBase imple
6464
get disabled(): boolean;
6565
set disabled(value: boolean);
6666
focus(options?: FocusOptions, origin?: FocusOrigin): void;
67+
_getInputTabindex(): number;
6768
id: string;
6869
_inputElement: ElementRef<HTMLInputElement>;
6970
get inputId(): string;

0 commit comments

Comments
 (0)