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.

Author: crisbeto

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

@@ -5,7 +5,7 @@
            [id]="inputId"
            [checked]="checked"
            [disabled]="disabled"
-           [tabIndex]="tabIndex"
+           [tabIndex]="_getInputTabindex()"
            [attr.name]="name"
            [attr.value]="value"
            [required]="required"

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

@@ -26,6 +26,7 @@ describe('MDC-based MatRadio', () => {
         TranscludingWrapper,
         RadioButtonWithPredefinedTabindex,
         RadioButtonWithPredefinedAriaAttributes,
+        RadiosInsidePreCheckedRadioGroup,
       ]
     });
 
@@ -402,6 +403,41 @@ describe('MDC-based MatRadio', () => {
           .every(element => element.classList.contains('mat-mdc-focus-indicator'))).toBe(true);
     });
 
+    it('should set the input tabindex based on the selected radio button', () => {
+      const getTabIndexes = () => {
+        return radioInputElements.map(element => parseInt(element.getAttribute('tabindex') || ''));
+      };
+
+      expect(getTabIndexes()).toEqual([0, 0, 0]);
+
+      radioLabelElements[0].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([0, -1, -1]);
+
+      radioLabelElements[1].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([-1, 0, -1]);
+
+      radioLabelElements[2].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([-1, -1, 0]);
+    });
+
+    it('should set the input tabindex correctly with a pre-checked radio button', () => {
+      const precheckedFixture = TestBed.createComponent(RadiosInsidePreCheckedRadioGroup);
+      precheckedFixture.detectChanges();
+
+      const radios: NodeListOf<HTMLElement> =
+          precheckedFixture.nativeElement.querySelectorAll('mat-radio-button input');
+
+      expect(Array.from(radios).map(radio => {
+        return radio.getAttribute('tabindex');
+      })).toEqual(['-1', '-1', '0']);
+    });
+
   });
 
   describe('group with ngModel', () => {
@@ -934,6 +970,19 @@ class RadiosInsideRadioGroup {
 }
 
 
+@Component({
+  template: `
+  <mat-radio-group name="test-name">
+    <mat-radio-button value="fire">Charmander</mat-radio-button>
+    <mat-radio-button value="water">Squirtle</mat-radio-button>
+    <mat-radio-button value="leaf" checked>Bulbasaur</mat-radio-button>
+  </mat-radio-group>
+  `
+})
+class RadiosInsidePreCheckedRadioGroup {
+}
+
+
 @Component({
   template: `
     <mat-radio-button name="season" value="spring">Spring</mat-radio-button>

src/material/radio/radio.html

@@ -9,7 +9,7 @@
         [id]="inputId"
         [checked]="checked"
         [disabled]="disabled"
-        [tabIndex]="tabIndex"
+        [tabIndex]="_getInputTabindex()"
         [attr.name]="name"
         [attr.value]="value"
         [required]="required"

src/material/radio/radio.spec.ts

@@ -23,6 +23,7 @@ describe('MatRadio', () => {
         TranscludingWrapper,
         RadioButtonWithPredefinedTabindex,
         RadioButtonWithPredefinedAriaAttributes,
+        RadiosInsidePreCheckedRadioGroup,
       ]
     });
 
@@ -395,6 +396,41 @@ describe('MatRadio', () => {
           .every(element => element.classList.contains('mat-focus-indicator'))).toBe(true);
     });
 
+    it('should set the input tabindex based on the selected radio button', () => {
+      const getTabIndexes = () => {
+        return radioInputElements.map(element => parseInt(element.getAttribute('tabindex') || ''));
+      };
+
+      expect(getTabIndexes()).toEqual([0, 0, 0]);
+
+      radioLabelElements[0].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([0, -1, -1]);
+
+      radioLabelElements[1].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([-1, 0, -1]);
+
+      radioLabelElements[2].click();
+      fixture.detectChanges();
+
+      expect(getTabIndexes()).toEqual([-1, -1, 0]);
+    });
+
+    it('should set the input tabindex correctly with a pre-checked radio button', () => {
+      const precheckedFixture = TestBed.createComponent(RadiosInsidePreCheckedRadioGroup);
+      precheckedFixture.detectChanges();
+
+      const radios: NodeListOf<HTMLElement> =
+          precheckedFixture.nativeElement.querySelectorAll('mat-radio-button input');
+
+      expect(Array.from(radios).map(radio => {
+        return radio.getAttribute('tabindex');
+      })).toEqual(['-1', '-1', '0']);
+    });
+
   });
 
   describe('group with ngModel', () => {
@@ -908,6 +944,18 @@ class RadiosInsideRadioGroup {
   color: string | null;
 }
 
+@Component({
+  template: `
+  <mat-radio-group name="test-name">
+    <mat-radio-button value="fire">Charmander</mat-radio-button>
+    <mat-radio-button value="water">Squirtle</mat-radio-button>
+    <mat-radio-button value="leaf" checked>Bulbasaur</mat-radio-button>
+  </mat-radio-group>
+  `
+})
+class RadiosInsidePreCheckedRadioGroup {
+}
+
 
 @Component({
   template: `

src/material/radio/radio.ts

@@ -199,6 +199,7 @@ export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase> implemen
     this._selected = selected;
     this.value = selected ? selected.value : null;
     this._checkSelectedRadioButton();
+    this._markRadiosForCheck();
   }
 
   /** Whether the radio group is disabled */
@@ -614,6 +615,21 @@ export abstract class _MatRadioButtonBase extends _MatRadioButtonMixinBase imple
     }
   }
 
+  /** Gets the tabindex for the underlying input element. */
+  _getInputTabindex(): number {
+    const group = this.radioGroup;
+
+    // Implement a roving tabindex if the button is inside a group. For most cases this isn't
+    // necessary, because the browser handles the tab order for inputs inside a group automatically,
+    // but we need an explicitly higher tabindex for the selected button in order for things like
+    // the focus trap to pick it up correctly.
+    if (!group || !group.selected || this.disabled) {
+      return this.tabIndex;
+    }
+
+    return group.selected === this ? this.tabIndex : -1;
+  }
+
   static ngAcceptInputType_checked: BooleanInput;
   static ngAcceptInputType_disabled: BooleanInput;
   static ngAcceptInputType_required: BooleanInput;

tools/public_api_guard/material/radio.d.ts

@@ -23,6 +23,7 @@ export declare abstract class _MatRadioButtonBase extends _MatRadioButtonMixinBa
     get value(): any;
     set value(value: any);
     constructor(radioGroup: _MatRadioGroupBase<_MatRadioButtonBase>, elementRef: ElementRef, _changeDetector: ChangeDetectorRef, _focusMonitor: FocusMonitor, _radioDispatcher: UniqueSelectionDispatcher, _animationMode?: string | undefined, _providerOverride?: MatRadioDefaultOptions | undefined, tabIndex?: string);
+    _getInputTabindex(): number;
     _isRippleDisabled(): boolean;
     _markForCheck(): void;
     _onInputChange(event: Event): void;