From 8e7a43d33b50d29b7bf4d668a875f29b422b3af0 Mon Sep 17 00:00:00 2001 From: naaajii Date: Sun, 2 Feb 2025 14:51:17 +0500 Subject: [PATCH] fix(material/radio): required attribute being set on buttons fixes when `MatRadioGroup` is set to be required it was setting all of its `MatRadioButton` to be required as-well which is confusing for assistive technologies, this commit ensures we only set aria-required on group rather than all buttons unless button is being used standalone of `MatRadioGroup` fixes #30399 --- src/material/radio/radio.html | 2 +- src/material/radio/radio.spec.ts | 27 +++++++++++++++++++ src/material/radio/radio.ts | 10 +++++++ .../radio/testing/radio-harness.spec.ts | 26 +++++++++++++++--- tools/public_api_guard/material/radio.md | 2 ++ 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/material/radio/radio.html b/src/material/radio/radio.html index 796d7e8708c0..5774b6b3fab1 100644 --- a/src/material/radio/radio.html +++ b/src/material/radio/radio.html @@ -8,7 +8,7 @@ [disabled]="disabled && !disabledInteractive" [attr.name]="name" [attr.value]="value" - [required]="required" + [required]="getInputRequiredAttribute()" [attr.aria-label]="ariaLabel" [attr.aria-labelledby]="ariaLabelledby" [attr.aria-describedby]="ariaDescribedby" diff --git a/src/material/radio/radio.spec.ts b/src/material/radio/radio.spec.ts index 0d2e0ccb5af4..c6e5be105069 100644 --- a/src/material/radio/radio.spec.ts +++ b/src/material/radio/radio.spec.ts @@ -524,6 +524,33 @@ describe('MatRadio', () => { expect(groupInstance.selected).toBe(null); expect(groupInstance.value).toBe('fire'); }); + + it('should set aria-required on radio group', () => { + fixture.changeDetectorRef.markForCheck(); + fixture.detectChanges(); + + let group = fixture.debugElement.query(By.directive(MatRadioGroup)).nativeElement; + + // by default it shouldn't be there + expect(group.getAttribute('aria-required')).toBe('false'); + + testComponent.isGroupRequired = true; + fixture.changeDetectorRef.markForCheck(); + fixture.detectChanges(); + + group = fixture.debugElement.query(By.directive(MatRadioGroup)).nativeElement; + expect(group.getAttribute('aria-required')).toBe('true'); + }); + + it('should set not set required attribute on matRadioButton when matRadioGroup is required', () => { + testComponent.isGroupRequired = true; + fixture.changeDetectorRef.markForCheck(); + fixture.detectChanges(); + + for (const radio of radioDebugElements) { + expect(radio.nativeElement.hasAttribute('aria-required')).toBeFalse(); + } + }); }); describe('group with ngModel', () => { diff --git a/src/material/radio/radio.ts b/src/material/radio/radio.ts index 99b54821b07d..d8bb198029d1 100644 --- a/src/material/radio/radio.ts +++ b/src/material/radio/radio.ts @@ -118,6 +118,7 @@ export function MAT_RADIO_DEFAULT_OPTIONS_FACTORY(): MatRadioDefaultOptions { host: { 'role': 'radiogroup', 'class': 'mat-mdc-radio-group', + '[attr.aria-required]': 'required', }, }) export class MatRadioGroup implements AfterContentInit, OnDestroy, ControlValueAccessor { @@ -804,4 +805,13 @@ export class MatRadioButton implements OnInit, AfterViewInit, DoCheck, OnDestroy } } } + + protected getInputRequiredAttribute(): boolean | null { + // we never want to set required attribute on input when we have MatRadioGroup as we will set + // aria-required directly on MatRadioGroup if its required as setting on all MatRadioButton for + // it's MatRadioGroup would be confusing for assistive technology. + if (this.radioGroup) return null; + + return this.required; + } } diff --git a/src/material/radio/testing/radio-harness.spec.ts b/src/material/radio/testing/radio-harness.spec.ts index 929a1f5b8537..61d0dc80c168 100644 --- a/src/material/radio/testing/radio-harness.spec.ts +++ b/src/material/radio/testing/radio-harness.spec.ts @@ -145,7 +145,7 @@ describe('radio harness', () => { describe('MatRadioButtonHarness', () => { it('should load all radio-button harnesses', async () => { const radios = await loader.getAllHarnesses(MatRadioButtonHarness); - expect(radios.length).toBe(9); + expect(radios.length).toBe(10); }); it('should load radio-button with exact label', async () => { @@ -267,10 +267,25 @@ describe('radio harness', () => { expect(await radioButton.isChecked()).toBe(true); }); - it('should get required state', async () => { + // radios with group should not contain required attribute as group itself is marked if its + // required or not, see #30399 + it('should have falsy required state if used with MatRadioGroup', async () => { + const radioButton = await loader.getHarness( + MatRadioButtonHarness.with({selector: '#required-radio-inside-group'}), + ); + expect(await radioButton.isRequired()).toBe(false); + }); + + it('should set required state of radio without group', async () => { const radioButton = await loader.getHarness( MatRadioButtonHarness.with({selector: '#required-radio'}), ); + expect(await radioButton.isRequired()).toBe(false); + + fixture.componentInstance.standaloneRequiredRadio = true; + fixture.changeDetectorRef.markForCheck(); + fixture.detectChanges(); + expect(await radioButton.isRequired()).toBe(true); }); }); @@ -302,11 +317,15 @@ describe('radio harness', () => { - + Accept terms of conditions + + Accept terms of conditions + + First @@ -321,4 +340,5 @@ class MultipleRadioButtonsHarnessTest { secondGroupId = 'my-group-2'; thirdGroupName: string = 'third-group-name'; thirdGroupButtonName: string | undefined = undefined; + standaloneRequiredRadio = false; } diff --git a/tools/public_api_guard/material/radio.md b/tools/public_api_guard/material/radio.md index 4a469a7294b6..645e25e66ed4 100644 --- a/tools/public_api_guard/material/radio.md +++ b/tools/public_api_guard/material/radio.md @@ -50,6 +50,8 @@ export class MatRadioButton implements OnInit, AfterViewInit, DoCheck, OnDestroy // (undocumented) protected _elementRef: ElementRef; focus(options?: FocusOptions, origin?: FocusOrigin): void; + // (undocumented) + protected getInputRequiredAttribute(): boolean | null; id: string; _inputElement: ElementRef; get inputId(): string;