Skip to content

Commit 8e7a43d

Browse files
committed
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
1 parent 1b3c42e commit 8e7a43d

File tree

5 files changed

+63
-4
lines changed

5 files changed

+63
-4
lines changed

src/material/radio/radio.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
[disabled]="disabled && !disabledInteractive"
99
[attr.name]="name"
1010
[attr.value]="value"
11-
[required]="required"
11+
[required]="getInputRequiredAttribute()"
1212
[attr.aria-label]="ariaLabel"
1313
[attr.aria-labelledby]="ariaLabelledby"
1414
[attr.aria-describedby]="ariaDescribedby"

src/material/radio/radio.spec.ts

+27
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,33 @@ describe('MatRadio', () => {
524524
expect(groupInstance.selected).toBe(null);
525525
expect(groupInstance.value).toBe('fire');
526526
});
527+
528+
it('should set aria-required on radio group', () => {
529+
fixture.changeDetectorRef.markForCheck();
530+
fixture.detectChanges();
531+
532+
let group = fixture.debugElement.query(By.directive(MatRadioGroup)).nativeElement;
533+
534+
// by default it shouldn't be there
535+
expect(group.getAttribute('aria-required')).toBe('false');
536+
537+
testComponent.isGroupRequired = true;
538+
fixture.changeDetectorRef.markForCheck();
539+
fixture.detectChanges();
540+
541+
group = fixture.debugElement.query(By.directive(MatRadioGroup)).nativeElement;
542+
expect(group.getAttribute('aria-required')).toBe('true');
543+
});
544+
545+
it('should set not set required attribute on matRadioButton when matRadioGroup is required', () => {
546+
testComponent.isGroupRequired = true;
547+
fixture.changeDetectorRef.markForCheck();
548+
fixture.detectChanges();
549+
550+
for (const radio of radioDebugElements) {
551+
expect(radio.nativeElement.hasAttribute('aria-required')).toBeFalse();
552+
}
553+
});
527554
});
528555

529556
describe('group with ngModel', () => {

src/material/radio/radio.ts

+10
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export function MAT_RADIO_DEFAULT_OPTIONS_FACTORY(): MatRadioDefaultOptions {
118118
host: {
119119
'role': 'radiogroup',
120120
'class': 'mat-mdc-radio-group',
121+
'[attr.aria-required]': 'required',
121122
},
122123
})
123124
export class MatRadioGroup implements AfterContentInit, OnDestroy, ControlValueAccessor {
@@ -804,4 +805,13 @@ export class MatRadioButton implements OnInit, AfterViewInit, DoCheck, OnDestroy
804805
}
805806
}
806807
}
808+
809+
protected getInputRequiredAttribute(): boolean | null {
810+
// we never want to set required attribute on input when we have MatRadioGroup as we will set
811+
// aria-required directly on MatRadioGroup if its required as setting on all MatRadioButton for
812+
// it's MatRadioGroup would be confusing for assistive technology.
813+
if (this.radioGroup) return null;
814+
815+
return this.required;
816+
}
807817
}

src/material/radio/testing/radio-harness.spec.ts

+23-3
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ describe('radio harness', () => {
145145
describe('MatRadioButtonHarness', () => {
146146
it('should load all radio-button harnesses', async () => {
147147
const radios = await loader.getAllHarnesses(MatRadioButtonHarness);
148-
expect(radios.length).toBe(9);
148+
expect(radios.length).toBe(10);
149149
});
150150

151151
it('should load radio-button with exact label', async () => {
@@ -267,10 +267,25 @@ describe('radio harness', () => {
267267
expect(await radioButton.isChecked()).toBe(true);
268268
});
269269

270-
it('should get required state', async () => {
270+
// radios with group should not contain required attribute as group itself is marked if its
271+
// required or not, see #30399
272+
it('should have falsy required state if used with MatRadioGroup', async () => {
273+
const radioButton = await loader.getHarness(
274+
MatRadioButtonHarness.with({selector: '#required-radio-inside-group'}),
275+
);
276+
expect(await radioButton.isRequired()).toBe(false);
277+
});
278+
279+
it('should set required state of radio without group', async () => {
271280
const radioButton = await loader.getHarness(
272281
MatRadioButtonHarness.with({selector: '#required-radio'}),
273282
);
283+
expect(await radioButton.isRequired()).toBe(false);
284+
285+
fixture.componentInstance.standaloneRequiredRadio = true;
286+
fixture.changeDetectorRef.markForCheck();
287+
fixture.detectChanges();
288+
274289
expect(await radioButton.isRequired()).toBe(true);
275290
});
276291
});
@@ -302,11 +317,15 @@ describe('radio harness', () => {
302317
303318
304319
<mat-radio-group [id]="secondGroupId" [name]="secondGroupId + '-name'">
305-
<mat-radio-button id="required-radio" required [value]="true">
320+
<mat-radio-button id="required-radio-inside-group" required [value]="true">
306321
Accept terms of conditions
307322
</mat-radio-button>
308323
</mat-radio-group>
309324
325+
<mat-radio-button id="required-radio" [required]="standaloneRequiredRadio" [value]="true">
326+
Accept terms of conditions
327+
</mat-radio-button>
328+
310329
<mat-radio-group [name]="thirdGroupName">
311330
<mat-radio-button [value]="true">First</mat-radio-button>
312331
<mat-radio-button [value]="false" [name]="thirdGroupButtonName"></mat-radio-button>
@@ -321,4 +340,5 @@ class MultipleRadioButtonsHarnessTest {
321340
secondGroupId = 'my-group-2';
322341
thirdGroupName: string = 'third-group-name';
323342
thirdGroupButtonName: string | undefined = undefined;
343+
standaloneRequiredRadio = false;
324344
}

tools/public_api_guard/material/radio.md

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ export class MatRadioButton implements OnInit, AfterViewInit, DoCheck, OnDestroy
5050
// (undocumented)
5151
protected _elementRef: ElementRef<any>;
5252
focus(options?: FocusOptions, origin?: FocusOrigin): void;
53+
// (undocumented)
54+
protected getInputRequiredAttribute(): boolean | null;
5355
id: string;
5456
_inputElement: ElementRef<HTMLInputElement>;
5557
get inputId(): string;

0 commit comments

Comments
 (0)