From 0d060c5ae942c2d4ca46a1fbb2eb138a6cd4101c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 16 Jul 2023 12:47:35 +0200 Subject: [PATCH] fix(material/radio): clear selected radio button from group In #18081 the radio group was changed so that deselected buttons receive `tabindex="-1"` when there's a selected button. The problem is that we weren't clearing the reference to the selected button so it gets removed, the deselected buttons become unfocusable using the keyboard. These changes clear the selected radio button on destroy. Fixes #27461. --- src/material/radio/radio.spec.ts | 24 +++++++++++++++++++----- src/material/radio/radio.ts | 20 +++++++++++++++++++- tools/public_api_guard/material/radio.md | 4 +++- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/material/radio/radio.spec.ts b/src/material/radio/radio.spec.ts index 5aec8aef5b85..1e32fc0f1cad 100644 --- a/src/material/radio/radio.spec.ts +++ b/src/material/radio/radio.spec.ts @@ -471,6 +471,19 @@ describe('MDC-based MatRadio', () => { }), ).toEqual(['-1', '-1', '0']); }); + + it('should clear the selected radio button but preserve the value on destroy', () => { + radioLabelElements[0].click(); + fixture.detectChanges(); + expect(groupInstance.selected).toBe(radioInstances[0]); + expect(groupInstance.value).toBe('fire'); + + fixture.componentInstance.isFirstShown = false; + fixture.detectChanges(); + + expect(groupInstance.selected).toBe(null); + expect(groupInstance.value).toBe('fire'); + }); }); describe('group with ngModel', () => { @@ -995,7 +1008,7 @@ describe('MatRadioDefaultOverrides', () => { [value]="groupValue" name="test-name"> + [color]="color" *ngIf="isFirstShown"> Charmander @@ -1009,12 +1022,13 @@ describe('MatRadioDefaultOverrides', () => { }) class RadiosInsideRadioGroup { labelPos: 'before' | 'after'; - isFirstDisabled: boolean = false; - isGroupDisabled: boolean = false; - isGroupRequired: boolean = false; + isFirstDisabled = false; + isGroupDisabled = false; + isGroupRequired = false; groupValue: string | null = null; - disableRipple: boolean = false; + disableRipple = false; color: string | null; + isFirstShown = true; } @Component({ diff --git a/src/material/radio/radio.ts b/src/material/radio/radio.ts index 367bfbebbb86..72aa7cb0e666 100644 --- a/src/material/radio/radio.ts +++ b/src/material/radio/radio.ts @@ -42,6 +42,7 @@ import {BooleanInput, coerceBooleanProperty, coerceNumberProperty} from '@angula import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; +import {Subscription} from 'rxjs'; // Increasing integer for generating unique ids for radio components. let nextUniqueId = 0; @@ -100,7 +101,7 @@ export function MAT_RADIO_DEFAULT_OPTIONS_FACTORY(): MatRadioDefaultOptions { */ @Directive() export abstract class _MatRadioGroupBase - implements AfterContentInit, ControlValueAccessor + implements AfterContentInit, OnDestroy, ControlValueAccessor { /** Selected value for the radio group. */ private _value: any = null; @@ -123,6 +124,9 @@ export abstract class _MatRadioGroupBase /** Whether the radio group is required. */ private _required: boolean = false; + /** Subscription to changes in amount of radio buttons. */ + private _buttonChanges: Subscription; + /** The method to be called in order to update ngModel */ _controlValueAccessorChangeFn: (value: any) => void = () => {}; @@ -236,6 +240,20 @@ export abstract class _MatRadioGroupBase // possibly be set by NgModel on MatRadioGroup, and it is possible that the OnInit of the // NgModel occurs *after* the OnInit of the MatRadioGroup. this._isInitialized = true; + + // Clear the `selected` button when it's destroyed since the tabindex of the rest of the + // buttons depends on it. Note that we don't clear the `value`, because the radio button + // may be swapped out with a similar one and there are some internal apps that depend on + // that behavior. + this._buttonChanges = this._radios.changes.subscribe(() => { + if (this.selected && !this._radios.find(radio => radio === this.selected)) { + this._selected = null; + } + }); + } + + ngOnDestroy() { + this._buttonChanges?.unsubscribe(); } /** diff --git a/tools/public_api_guard/material/radio.md b/tools/public_api_guard/material/radio.md index fc19d0686502..5eade5af5fb8 100644 --- a/tools/public_api_guard/material/radio.md +++ b/tools/public_api_guard/material/radio.md @@ -124,7 +124,7 @@ export class MatRadioGroup extends _MatRadioGroupBase { } // @public -export abstract class _MatRadioGroupBase implements AfterContentInit, ControlValueAccessor { +export abstract class _MatRadioGroupBase implements AfterContentInit, OnDestroy, ControlValueAccessor { constructor(_changeDetector: ChangeDetectorRef); readonly change: EventEmitter; // (undocumented) @@ -141,6 +141,8 @@ export abstract class _MatRadioGroupBase implemen get name(): string; set name(value: string); ngAfterContentInit(): void; + // (undocumented) + ngOnDestroy(): void; onTouched: () => any; abstract _radios: QueryList; registerOnChange(fn: (value: any) => void): void;