Skip to content

Commit

Permalink
fix(material/chips): don't remove aria-selected from deselected optio…
Browse files Browse the repository at this point in the history
…ns (#25748)

For chip-listbox, set `aria-selected="false"` on deselected options that
are selectable. Conforms with [WAI ARIA Listbox authoring practices guide](
https://www.w3.org/WAI/ARIA/apg/patterns/listbox/), which says to always
include aria-selected attribute on options that are selectable. Fix issue where
voiceover reads every option as "selected" (#25736).

Fix #25736
  • Loading branch information
zarend committed Oct 7, 2022
1 parent 87e17aa commit 147a354
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
16 changes: 8 additions & 8 deletions src/material/chips/chip-listbox.spec.ts
Expand Up @@ -513,19 +513,19 @@ describe('MDC-based MatChipListbox', () => {
'.mdc-evolution-chip__action--primary',
);

expect(getAriaSelected()).toEqual([null, null, null]);
expect(getAriaSelected()).toEqual(['false', 'false', 'false']);

primaryActions[1].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual([null, 'true', null]);
expect(getAriaSelected()).toEqual(['false', 'true', 'false']);

primaryActions[2].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual([null, null, 'true']);
expect(getAriaSelected()).toEqual(['false', 'false', 'true']);

primaryActions[0].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual(['true', null, null]);
expect(getAriaSelected()).toEqual(['true', 'false', 'false']);
}));

it('should set `aria-selected` based on the selection state in multi-selection mode', fakeAsync(() => {
Expand All @@ -548,23 +548,23 @@ describe('MDC-based MatChipListbox', () => {
'.mdc-evolution-chip__action--primary',
);

expect(getAriaSelected()).toEqual([null, null, null]);
expect(getAriaSelected()).toEqual(['false', 'false', 'false']);

primaryActions[1].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual([null, 'true', null]);
expect(getAriaSelected()).toEqual(['false', 'true', 'false']);

primaryActions[2].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual([null, 'true', 'true']);
expect(getAriaSelected()).toEqual(['false', 'true', 'true']);

primaryActions[0].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual(['true', 'true', 'true']);

primaryActions[1].click();
fixture.detectChanges();
expect(getAriaSelected()).toEqual(['true', null, 'true']);
expect(getAriaSelected()).toEqual(['true', 'false', 'true']);
}));
});

Expand Down
2 changes: 1 addition & 1 deletion src/material/chips/chip-option.spec.ts
Expand Up @@ -233,7 +233,7 @@ describe('MDC-based Option Chips', () => {
});

it('should have correct aria-selected in single selection mode', () => {
expect(primaryAction.hasAttribute('aria-selected')).toBe(false);
expect(primaryAction.getAttribute('aria-selected')).toBe('false');

testComponent.selected = true;
fixture.detectChanges();
Expand Down
20 changes: 14 additions & 6 deletions src/material/chips/chip-option.ts
Expand Up @@ -106,13 +106,21 @@ export class MatChipOption extends MatChip implements OnInit {
}
private _selected = false;

/** The ARIA selected applied to the chip. */
/**
* The ARIA selected applied to the chip. Conforms to WAI ARIA best practices for listbox
* interaction patterns.
*
* From [WAI ARIA Listbox authoring practices guide](
* https://www.w3.org/WAI/ARIA/apg/patterns/listbox/):
* "If any options are selected, each selected option has either aria-selected or aria-checked
* set to true. All options that are selectable but not selected have either aria-selected or
* aria-checked set to false."
*
* Set `aria-selected="false"` on not-selected listbox options that are selectable to fix
* VoiceOver reading every option as "selected" (#25736).
*/
get ariaSelected(): string | null {
// Remove the `aria-selected` when the chip is deselected in single-selection mode, because
// it adds noise to NVDA users where "not selected" will be read out for each chip.
return this.selectable && (this._chipListMultiple || this.selected)
? this.selected.toString()
: null;
return this.selectable ? this.selected.toString() : null;
}

/** The unstyled chip selector for this component. */
Expand Down

0 comments on commit 147a354

Please sign in to comment.