fix(material/button-toggle): unable to tab into ngModel-based group on first render (angular#30103)

Fixes an issue where the button toggle group would reset all the buttons back to `tabindex="-1"` if they're all static (e.g. not in a loop) and none of them match the value.

Fixes angular#30097.

Author: crisbeto

src/material/button-toggle/button-toggle.spec.ts

@@ -96,8 +96,6 @@ describe('MatButtonToggle with forms', () => {
       innerButtons = buttonToggleDebugElements.map(
         debugEl => debugEl.query(By.css('button'))!.nativeElement,
       );
-
-      fixture.detectChanges();
     });
 
     it('should update the model before firing change event', fakeAsync(() => {
@@ -312,6 +310,18 @@ describe('MatButtonToggle with forms', () => {
 
     expect(instance.toggles.map(t => t.checked)).toEqual([true, false, false]);
   });
+
+  it('should set the initial tabindex when using ngModel with a static list of options where none match the value', fakeAsync(() => {
+    const fixture = TestBed.createComponent(ButtonToggleGroupWithNgModelAndStaticOptions);
+    fixture.detectChanges();
+    tick();
+    const indexes = Array.from(
+      fixture.nativeElement.querySelectorAll('button'),
+      (button: HTMLElement) => button.getAttribute('tabindex'),
+    );
+
+    expect(indexes).toEqual(['0', '-1', '-1']);
+  }));
 });
 
 describe('MatButtonToggle without forms', () => {
@@ -341,7 +351,7 @@ describe('MatButtonToggle without forms', () => {
     let groupNativeElement: HTMLElement;
     let buttonToggleDebugElements: DebugElement[];
     let buttonToggleNativeElements: HTMLElement[];
-    let buttonToggleLabelElements: HTMLLabelElement[];
+    let innerButtons: HTMLLabelElement[];
     let groupInstance: MatButtonToggleGroup;
     let buttonToggleInstances: MatButtonToggle[];
     let testComponent: ButtonTogglesInsideButtonToggleGroup;
@@ -360,34 +370,28 @@ describe('MatButtonToggle without forms', () => {
 
       buttonToggleNativeElements = buttonToggleDebugElements.map(debugEl => debugEl.nativeElement);
 
-      buttonToggleLabelElements = fixture.debugElement
+      innerButtons = fixture.debugElement
         .queryAll(By.css('button'))
         .map(debugEl => debugEl.nativeElement);
 
       buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
     });
 
     it('should initialize the tab index correctly', () => {
-      buttonToggleLabelElements.forEach((buttonToggle, index) => {
-        if (index === 0) {
-          expect(buttonToggle.getAttribute('tabindex')).toBe('0');
-        } else {
-          expect(buttonToggle.getAttribute('tabindex')).toBe('-1');
-        }
-      });
+      expect(innerButtons.map(b => b.getAttribute('tabindex'))).toEqual(['0', '-1', '-1']);
     });
 
     it('should update the tab index correctly', () => {
-      buttonToggleLabelElements[1].click();
+      innerButtons[1].click();
       fixture.detectChanges();
 
-      expect(buttonToggleLabelElements[0].getAttribute('tabindex')).toBe('-1');
-      expect(buttonToggleLabelElements[1].getAttribute('tabindex')).toBe('0');
+      expect(innerButtons[0].getAttribute('tabindex')).toBe('-1');
+      expect(innerButtons[1].getAttribute('tabindex')).toBe('0');
     });
 
     it('should set individual button toggle names based on the group name', () => {
       expect(groupInstance.name).toBeTruthy();
-      for (let buttonToggle of buttonToggleLabelElements) {
+      for (let buttonToggle of innerButtons) {
         expect(buttonToggle.getAttribute('name')).toBe(groupInstance.name);
       }
     });
@@ -407,7 +411,7 @@ describe('MatButtonToggle without forms', () => {
 
       expect(buttonToggleInstances[0].disabled).toBe(false);
 
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(buttonToggleInstances[0].checked).toBe(true);
@@ -456,7 +460,7 @@ describe('MatButtonToggle without forms', () => {
 
     it('should update the group value when one of the toggles changes', () => {
       expect(groupInstance.value).toBeFalsy();
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
@@ -465,7 +469,7 @@ describe('MatButtonToggle without forms', () => {
 
     it('should propagate the value change back up via a two-way binding', () => {
       expect(groupInstance.value).toBeFalsy();
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
@@ -474,15 +478,15 @@ describe('MatButtonToggle without forms', () => {
 
     it('should update the group and toggles when one of the button toggles is clicked', () => {
       expect(groupInstance.value).toBeFalsy();
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
       expect(groupInstance.selected).toBe(buttonToggleInstances[0]);
       expect(buttonToggleInstances[0].checked).toBe(true);
       expect(buttonToggleInstances[1].checked).toBe(false);
 
-      buttonToggleLabelElements[1].click();
+      innerButtons[1].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test2');
@@ -492,7 +496,7 @@ describe('MatButtonToggle without forms', () => {
     });
 
     it('should check a button toggle upon interaction with underlying native radio button', () => {
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(buttonToggleInstances[0].checked).toBe(true);
@@ -515,12 +519,12 @@ describe('MatButtonToggle without forms', () => {
       const changeSpy = jasmine.createSpy('button-toggle change listener');
       buttonToggleInstances[0].change.subscribe(changeSpy);
 
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalledTimes(1);
 
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
       tick();
 
@@ -534,12 +538,12 @@ describe('MatButtonToggle without forms', () => {
       const changeSpy = jasmine.createSpy('button-toggle-group change listener');
       groupInstance.change.subscribe(changeSpy);
 
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalled();
 
-      buttonToggleLabelElements[1].click();
+      innerButtons[1].click();
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalledTimes(2);
@@ -579,7 +583,7 @@ describe('MatButtonToggle without forms', () => {
 
     it('should update the model if a selected toggle is removed', fakeAsync(() => {
       expect(groupInstance.value).toBeFalsy();
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(groupInstance.value).toBe('test1');
@@ -595,7 +599,7 @@ describe('MatButtonToggle without forms', () => {
     }));
 
     it('should show checkmark indicator by default', () => {
-      buttonToggleLabelElements[0].click();
+      innerButtons[0].click();
       fixture.detectChanges();
 
       expect(
@@ -1310,3 +1314,18 @@ class ButtonToggleGroupWithFormControlAndDynamicButtons {
   control = new FormControl('');
   values = ['a', 'b', 'c'];
 }
+
+@Component({
+  template: `
+    <mat-button-toggle-group [(ngModel)]="value">
+      <mat-button-toggle value="1">One</mat-button-toggle>
+      <mat-button-toggle value="2">Two</mat-button-toggle>
+      <mat-button-toggle value="3">Three</mat-button-toggle>
+    </mat-button-toggle-group>
+  `,
+  standalone: true,
+  imports: [MatButtonToggleModule, FormsModule],
+})
+class ButtonToggleGroupWithNgModelAndStaticOptions {
+  value = '';
+}

src/material/button-toggle/button-toggle.ts

@@ -473,16 +473,28 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
       return;
     }
 
+    const toggles = this._buttonToggles.toArray();
+
     if (this.multiple && value) {
       if (!Array.isArray(value) && (typeof ngDevMode === 'undefined' || ngDevMode)) {
         throw Error('Value must be an array in multiple-selection mode.');
       }
 
       this._clearSelection();
-      value.forEach((currentValue: any) => this._selectValue(currentValue));
+      value.forEach((currentValue: any) => this._selectValue(currentValue, toggles));
     } else {
       this._clearSelection();
-      this._selectValue(value);
+      this._selectValue(value, toggles);
+    }
+
+    // In single selection mode we need at least one enabled toggle to always be focusable.
+    if (!this.multiple && toggles.every(toggle => toggle.tabIndex === -1)) {
+      for (const toggle of toggles) {
+        if (!toggle.disabled) {
+          toggle.tabIndex = 0;
+          break;
+        }
+      }
     }
   }
 
@@ -499,17 +511,16 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
   }
 
   /** Selects a value if there's a toggle that corresponds to it. */
-  private _selectValue(value: any) {
-    const correspondingOption = this._buttonToggles.find(toggle => {
-      return toggle.value != null && toggle.value === value;
-    });
-
-    if (correspondingOption) {
-      correspondingOption.checked = true;
-      this._selectionModel.select(correspondingOption);
-      if (!this.multiple) {
-        // If the button toggle is in single select mode, reset the tabIndex.
-        correspondingOption.tabIndex = 0;
+  private _selectValue(value: any, toggles: MatButtonToggle[]) {
+    for (const toggle of toggles) {
+      if (toggle.value != null && toggle.value === value) {
+        toggle.checked = true;
+        this._selectionModel.select(toggle);
+        if (!this.multiple) {
+          // If the button toggle is in single select mode, reset the tabIndex.
+          toggle.tabIndex = 0;
+        }
+        break;
       }
     }
   }
@@ -601,8 +612,10 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
     return this._tabIndex;
   }
   set tabIndex(value: number | null) {
-    this._tabIndex = value;
-    this._markForCheck();
+    if (value !== this._tabIndex) {
+      this._tabIndex = value;
+      this._markForCheck();
+    }
   }
   private _tabIndex: number | null;
 
@@ -668,14 +681,13 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
   constructor() {
     inject(_CdkPrivateStyleLoader).load(_StructuralStylesLoader);
     const toggleGroup = inject<MatButtonToggleGroup>(MAT_BUTTON_TOGGLE_GROUP, {optional: true})!;
-    const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true});
+    const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true}) || '';
     const defaultOptions = inject<MatButtonToggleDefaultOptions>(
       MAT_BUTTON_TOGGLE_DEFAULT_OPTIONS,
       {optional: true},
     );
 
-    const parsedTabIndex = Number(defaultTabIndex);
-    this.tabIndex = parsedTabIndex || parsedTabIndex === 0 ? parsedTabIndex : null;
+    this._tabIndex = parseInt(defaultTabIndex) || 0;
     this.buttonToggleGroup = toggleGroup;
     this.appearance =
       defaultOptions && defaultOptions.appearance ? defaultOptions.appearance : 'standard';