Skip to content

Commit

Permalink
fix(material/radio): clear selected radio button from group (#27466)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 6c846e2)
  • Loading branch information
crisbeto committed Jul 17, 2023
1 parent 4d94731 commit 465a7b0
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 7 deletions.
24 changes: 19 additions & 5 deletions src/material/radio/radio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -995,7 +1008,7 @@ describe('MatRadioDefaultOverrides', () => {
[value]="groupValue"
name="test-name">
<mat-radio-button value="fire" [disableRipple]="disableRipple" [disabled]="isFirstDisabled"
[color]="color">
[color]="color" *ngIf="isFirstShown">
Charmander
</mat-radio-button>
<mat-radio-button value="water" [disableRipple]="disableRipple" [color]="color">
Expand All @@ -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({
Expand Down
20 changes: 19 additions & 1 deletion src/material/radio/radio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,7 +101,7 @@ export function MAT_RADIO_DEFAULT_OPTIONS_FACTORY(): MatRadioDefaultOptions {
*/
@Directive()
export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase>
implements AfterContentInit, ControlValueAccessor
implements AfterContentInit, OnDestroy, ControlValueAccessor
{
/** Selected value for the radio group. */
private _value: any = null;
Expand All @@ -123,6 +124,9 @@ export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase>
/** 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 = () => {};

Expand Down Expand Up @@ -236,6 +240,20 @@ export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase>
// 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();
}

/**
Expand Down
4 changes: 3 additions & 1 deletion tools/public_api_guard/material/radio.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export class MatRadioGroup extends _MatRadioGroupBase<MatRadioButton> {
}

// @public
export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase> implements AfterContentInit, ControlValueAccessor {
export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase> implements AfterContentInit, OnDestroy, ControlValueAccessor {
constructor(_changeDetector: ChangeDetectorRef);
readonly change: EventEmitter<MatRadioChange>;
// (undocumented)
Expand All @@ -141,6 +141,8 @@ export abstract class _MatRadioGroupBase<T extends _MatRadioButtonBase> implemen
get name(): string;
set name(value: string);
ngAfterContentInit(): void;
// (undocumented)
ngOnDestroy(): void;
onTouched: () => any;
abstract _radios: QueryList<T>;
registerOnChange(fn: (value: any) => void): void;
Expand Down

0 comments on commit 465a7b0

Please sign in to comment.