fix(material/button-toggle): toggle name falling out of sync if name changes (#24713)

Currently we assign the group name to each individual toggle on init or when the group name changes, but that doesn't account for when the toggle's name changes independently.

These changes switch to using a getter which will update automatically if anything changes.

Fixes #24666.

Author: crisbeto

src/material/button-toggle/button-toggle.html

@@ -4,7 +4,7 @@
         [attr.tabindex]="disabled ? -1 : tabIndex"
         [attr.aria-pressed]="checked"
         [disabled]="disabled || null"
-        [attr.name]="name || null"
+        [attr.name]="_getButtonName()"
         [attr.aria-label]="ariaLabel"
         [attr.aria-labelledby]="ariaLabelledby"
         (click)="_onButtonClick()">

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

@@ -113,13 +113,15 @@ describe('MatButtonToggle with forms', () => {
 
     it('should set individual radio names based on the group name', () => {
       expect(groupInstance.name).toBeTruthy();
-      for (let buttonToggle of buttonToggleInstances) {
-        expect(buttonToggle.name).toBe(groupInstance.name);
+      for (let buttonToggle of innerButtons) {
+        expect(buttonToggle.getAttribute('name')).toBe(groupInstance.name);
       }
 
       groupInstance.name = 'new name';
-      for (let buttonToggle of buttonToggleInstances) {
-        expect(buttonToggle.name).toBe(groupInstance.name);
+      fixture.detectChanges();
+
+      for (let buttonToggle of innerButtons) {
+        expect(buttonToggle.getAttribute('name')).toBe(groupInstance.name);
       }
     });
 
@@ -137,6 +139,15 @@ describe('MatButtonToggle with forms', () => {
         .toBe(true);
     });
 
+    it('should set the name of the buttons to the group name if the name of the button changes after init', () => {
+      const firstButton = innerButtons[0];
+      expect(firstButton.getAttribute('name')).toBe(fixture.componentInstance.groupName);
+
+      fixture.componentInstance.options[0].name = 'changed-name';
+      fixture.detectChanges();
+      expect(firstButton.getAttribute('name')).toBe(fixture.componentInstance.groupName);
+    });
+
     it('should check the corresponding button toggle on a group value change', () => {
       expect(groupInstance.value).toBeFalsy();
       for (let buttonToggle of buttonToggleInstances) {
@@ -313,8 +324,8 @@ describe('MatButtonToggle without forms', () => {
 
     it('should set individual button toggle names based on the group name', () => {
       expect(groupInstance.name).toBeTruthy();
-      for (let buttonToggle of buttonToggleInstances) {
-        expect(buttonToggle.name).toBe(groupInstance.name);
+      for (let buttonToggle of buttonToggleLabelElements) {
+        expect(buttonToggle.getAttribute('name')).toBe(groupInstance.name);
       }
     });
 
@@ -930,8 +941,10 @@ class ButtonTogglesInsideButtonToggleGroup {
     [name]="groupName"
     [(ngModel)]="modelValue"
     (change)="lastEvent = $event">
-    <mat-button-toggle *ngFor="let option of options" [value]="option.value"
-                       [disableRipple]="disableRipple">
+    <mat-button-toggle *ngFor="let option of options"
+      [value]="option.value"
+      [disableRipple]="disableRipple"
+      [name]="option.name">
       {{option.label}}
     </mat-button-toggle>
   </mat-button-toggle-group>
@@ -941,9 +954,9 @@ class ButtonToggleGroupWithNgModel {
   groupName = 'group-name';
   modelValue: string;
   options = [
-    {label: 'Red', value: 'red'},
-    {label: 'Green', value: 'green'},
-    {label: 'Blue', value: 'blue'},
+    {label: 'Red', value: 'red', name: ''},
+    {label: 'Green', value: 'green', name: ''},
+    {label: 'Blue', value: 'blue', name: ''},
   ];
   lastEvent: MatButtonToggleChange;
   disableRipple = false;

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

@@ -155,13 +155,7 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
   }
   set name(value: string) {
     this._name = value;
-
-    if (this._buttonToggles) {
-      this._buttonToggles.forEach(toggle => {
-        toggle.name = this._name;
-        toggle._markForCheck();
-      });
-    }
+    this._markButtonsForCheck();
   }
   private _name = `mat-button-toggle-group-${uniqueIdCounter++}`;
 
@@ -210,6 +204,7 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
   }
   set multiple(value: BooleanInput) {
     this._multiple = coerceBooleanProperty(value);
+    this._markButtonsForCheck();
   }
 
   /** Whether multiple button toggle group is disabled. */
@@ -219,10 +214,7 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
   }
   set disabled(value: BooleanInput) {
     this._disabled = coerceBooleanProperty(value);
-
-    if (this._buttonToggles) {
-      this._buttonToggles.forEach(toggle => toggle._markForCheck());
-    }
+    this._markButtonsForCheck();
   }
 
   /** Event emitted when the group's value changes. */
@@ -387,6 +379,11 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
     // it is used by Angular to sync up the two-way data binding.
     this.valueChange.emit(this.value);
   }
+
+  /** Marks all of the child button toggles to be checked. */
+  private _markButtonsForCheck() {
+    this._buttonToggles?.forEach(toggle => toggle._markForCheck());
+  }
 }
 
 // Boilerplate for applying mixins to the MatButtonToggle class.
@@ -420,7 +417,6 @@ export class MatButtonToggle
   extends _MatButtonToggleBase
   implements OnInit, AfterViewInit, CanDisableRipple, OnDestroy
 {
-  private _isSingleSelector = false;
   private _checked = false;
 
   /**
@@ -521,13 +517,8 @@ export class MatButtonToggle
 
   ngOnInit() {
     const group = this.buttonToggleGroup;
-    this._isSingleSelector = group && !group.multiple;
     this.id = this.id || `mat-button-toggle-${uniqueIdCounter++}`;
 
-    if (this._isSingleSelector) {
-      this.name = group.name;
-    }
-
     if (group) {
       if (group._isPrechecked(this)) {
         this.checked = true;
@@ -564,7 +555,7 @@ export class MatButtonToggle
 
   /** Checks the button toggle due to an interaction with the underlying native button. */
   _onButtonClick() {
-    const newChecked = this._isSingleSelector ? true : !this._checked;
+    const newChecked = this._isSingleSelector() ? true : !this._checked;
 
     if (newChecked !== this._checked) {
       this._checked = newChecked;
@@ -587,4 +578,17 @@ export class MatButtonToggle
     // Use `markForCheck` to explicit update button toggle's status.
     this._changeDetectorRef.markForCheck();
   }
+
+  /** Gets the name that should be assigned to the inner DOM node. */
+  _getButtonName(): string | null {
+    if (this._isSingleSelector()) {
+      return this.buttonToggleGroup.name;
+    }
+    return this.name || null;
+  }
+
+  /** Whether the toggle is in single selection mode. */
+  private _isSingleSelector(): boolean {
+    return this.buttonToggleGroup && !this.buttonToggleGroup.multiple;
+  }
 }

tools/public_api_guard/material/button-toggle.md

@@ -47,6 +47,7 @@ export class MatButtonToggle extends _MatButtonToggleBase implements OnInit, Aft
     get disabled(): boolean;
     set disabled(value: BooleanInput);
     focus(options?: FocusOptions): void;
+    _getButtonName(): string | null;
     id: string;
     _markForCheck(): void;
     name: string;