Skip to content

Commit

Permalink
fix(material/checkbox): fix ARIA semantics and use native DOM propert…
Browse files Browse the repository at this point in the history
…ies (#26710)

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
  • Loading branch information
zarend authored Mar 14, 2023
1 parent d444c58 commit 5c5617d
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/material/checkbox/checkbox.html
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 6 additions & 9 deletions src/material/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,16 @@ 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();
flush();

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();
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 0 additions & 8 deletions src/material/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,6 @@ export abstract class _MatCheckboxBase<E>
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();
Expand Down
1 change: 0 additions & 1 deletion src/material/legacy-checkbox/checkbox.html
Original file line number Diff line number Diff line change
Expand Up @@ -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)">
Expand Down
9 changes: 0 additions & 9 deletions src/material/legacy-checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,13 @@ 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();

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();
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions tools/public_api_guard/material/checkbox.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ export abstract class _MatCheckboxBase<E> 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;
Expand Down

0 comments on commit 5c5617d

Please sign in to comment.