From b5bf2c07689b679aca864a79cdaefb462040e4ad Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Fri, 24 Feb 2023 19:34:29 +0000 Subject: [PATCH] fix(material/checkbox): fix ARIA semantics and use native DOM properties For the checkbox component, fix semantics so that a11y produces a state of "mixed" when both the `checked` and `indeterminate` Inputs to mat-checkbox are both true. Also, remove the use of aria-checked in favor of the native DOM properties for checked and indeterminate. Conform to [ARIA in HTML W3C input checkbox specification](https://www.w3.org/TR/html-aria/#el-input-checkbox). Fix a11y bug where checkbox renders the indeterminate visual state but accessibility tree has a state of "checked". Aligns with native checkbox, which produces the mixed state when indeterminate property is true, regardless of the value of the checked property. Some accessibility checkers produce an error when native checkbox has aria-checked on it. Remove aria-checked to fix errors. Does not make visual changes. Fix #26709 --- src/material/checkbox/checkbox.html | 2 +- src/material/checkbox/checkbox.spec.ts | 15 ++++++--------- src/material/checkbox/checkbox.ts | 8 -------- src/material/legacy-checkbox/checkbox.html | 1 - src/material/legacy-checkbox/checkbox.spec.ts | 9 --------- tools/public_api_guard/material/checkbox.md | 2 -- 6 files changed, 7 insertions(+), 30 deletions(-) diff --git a/src/material/checkbox/checkbox.html b/src/material/checkbox/checkbox.html index 5c9cbab0416e..bad07f366bd9 100644 --- a/src/material/checkbox/checkbox.html +++ b/src/material/checkbox/checkbox.html @@ -8,13 +8,13 @@ type="checkbox" class="mdc-checkbox__native-control" [class.mdc-checkbox--selected]="checked" - [attr.aria-checked]="_getAriaChecked()" [attr.aria-label]="ariaLabel || null" [attr.aria-labelledby]="ariaLabelledby" [attr.aria-describedby]="ariaDescribedby" [attr.name]="name" [attr.value]="value" [checked]="checked" + [indeterminate]="indeterminate" [disabled]="disabled" [id]="inputId" [required]="required" diff --git a/src/material/checkbox/checkbox.spec.ts b/src/material/checkbox/checkbox.spec.ts index 4d6d39a76c1d..8942f597ffa2 100644 --- a/src/material/checkbox/checkbox.spec.ts +++ b/src/material/checkbox/checkbox.spec.ts @@ -96,9 +96,6 @@ describe('MDC-based MatCheckbox', () => { it('should add and remove indeterminate state', fakeAsync(() => { expect(inputElement.checked).toBe(false); expect(inputElement.indeterminate).toBe(false); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria-checked to be false') - .toBe('false'); testComponent.isIndeterminate = true; fixture.detectChanges(); @@ -106,9 +103,9 @@ describe('MDC-based MatCheckbox', () => { expect(inputElement.checked).toBe(false); expect(inputElement.indeterminate).toBe(true); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria checked to be mixed for indeterminate checkbox') - .toBe('mixed'); + expect(inputElement.hasAttribute('aria-checked')) + .withContext('Expect aria-checked attribute to not be used') + .toBe(false); testComponent.isIndeterminate = false; fixture.detectChanges(); @@ -148,9 +145,9 @@ describe('MDC-based MatCheckbox', () => { expect(inputElement.indeterminate).toBe(true); expect(inputElement.checked).toBe(true); expect(testComponent.isIndeterminate).toBe(true); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria checked to be true') - .toBe('true'); + expect(inputElement.hasAttribute('aria-checked')) + .withContext('Expect aria-checked attribute to not be used') + .toBe(false); inputElement.click(); fixture.detectChanges(); diff --git a/src/material/checkbox/checkbox.ts b/src/material/checkbox/checkbox.ts index 2f97a74ef393..afea3432f432 100644 --- a/src/material/checkbox/checkbox.ts +++ b/src/material/checkbox/checkbox.ts @@ -311,14 +311,6 @@ export abstract class _MatCheckboxBase this.disabled = isDisabled; } - _getAriaChecked(): 'true' | 'false' | 'mixed' { - if (this.checked) { - return 'true'; - } - - return this.indeterminate ? 'mixed' : 'false'; - } - private _transitionCheckState(newState: TransitionCheckState) { let oldState = this._currentCheckState; let element = this._getAnimationTargetElement(); diff --git a/src/material/legacy-checkbox/checkbox.html b/src/material/legacy-checkbox/checkbox.html index 22b400daac3c..4cdd3b374070 100644 --- a/src/material/legacy-checkbox/checkbox.html +++ b/src/material/legacy-checkbox/checkbox.html @@ -12,7 +12,6 @@ [tabIndex]="tabIndex" [attr.aria-label]="ariaLabel || null" [attr.aria-labelledby]="ariaLabelledby" - [attr.aria-checked]="_getAriaChecked()" [attr.aria-describedby]="ariaDescribedby" (change)="_onInteractionEvent($event)" (click)="_onInputClick($event)"> diff --git a/src/material/legacy-checkbox/checkbox.spec.ts b/src/material/legacy-checkbox/checkbox.spec.ts index 5038127f28cf..1241ae3d58f1 100644 --- a/src/material/legacy-checkbox/checkbox.spec.ts +++ b/src/material/legacy-checkbox/checkbox.spec.ts @@ -73,9 +73,6 @@ describe('MatLegacyCheckbox', () => { expect(checkboxNativeElement.classList).not.toContain('mat-checkbox-checked'); expect(inputElement.checked).toBe(false); expect(inputElement.indeterminate).toBe(false); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria-checked to be false') - .toBe('false'); testComponent.isIndeterminate = true; fixture.detectChanges(); @@ -83,9 +80,6 @@ describe('MatLegacyCheckbox', () => { expect(checkboxNativeElement.classList).toContain('mat-checkbox-indeterminate'); expect(inputElement.checked).toBe(false); expect(inputElement.indeterminate).toBe(true); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria checked to be mixed for indeterminate checkbox') - .toBe('mixed'); testComponent.isIndeterminate = false; fixture.detectChanges(); @@ -125,9 +119,6 @@ describe('MatLegacyCheckbox', () => { expect(inputElement.indeterminate).toBe(true); expect(inputElement.checked).toBe(true); expect(testComponent.isIndeterminate).toBe(true); - expect(inputElement.getAttribute('aria-checked')) - .withContext('Expect aria checked to be true') - .toBe('true'); inputElement.click(); fixture.detectChanges(); diff --git a/tools/public_api_guard/material/checkbox.md b/tools/public_api_guard/material/checkbox.md index 70a37345b4e8..2713e34a456a 100644 --- a/tools/public_api_guard/material/checkbox.md +++ b/tools/public_api_guard/material/checkbox.md @@ -94,8 +94,6 @@ export abstract class _MatCheckboxBase extends _MatCheckboxMixinBase implemen abstract focus(origin?: FocusOrigin): void; protected abstract _getAnimationTargetElement(): HTMLElement | null; // (undocumented) - _getAriaChecked(): 'true' | 'false' | 'mixed'; - // (undocumented) protected _handleInputClick(): void; id: string; get indeterminate(): boolean;