Skip to content

fix(chips): remove aria-selected from deselected chip in single selection mode #15634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/lib/chips/chip-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
get multiple(): boolean { return this._multiple; }
set multiple(value: boolean) {
this._multiple = coerceBooleanProperty(value);
this._syncChipsState();
}
private _multiple: boolean = false;

Expand Down Expand Up @@ -267,7 +268,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
get disabled(): boolean { return this.ngControl ? !!this.ngControl.disabled : this._disabled; }
set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);
this._syncChipsDisabledState();
this._syncChipsState();
}
protected _disabled: boolean = false;

Expand Down Expand Up @@ -371,7 +372,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
// Since this happens after the content has been
// checked, we need to defer it to the next tick.
Promise.resolve().then(() => {
this._syncChipsDisabledState();
this._syncChipsState();
});
}

Expand Down Expand Up @@ -781,11 +782,12 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
return this.chips.some(chip => chip._hasFocus);
}

/** Syncs the list's disabled state with the individual chips. */
private _syncChipsDisabledState() {
/** Syncs the list's state with the individual chips. */
private _syncChipsState() {
if (this.chips) {
this.chips.forEach(chip => {
chip.disabled = this._disabled;
chip._chipListMultiple = this.multiple;
});
}
}
Expand Down
20 changes: 17 additions & 3 deletions src/lib/chips/chip.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {Directionality} from '@angular/cdk/bidi';
import {BACKSPACE, DELETE, SPACE} from '@angular/cdk/keycodes';
import {createKeyboardEvent, dispatchFakeEvent} from '@angular/cdk/testing';
import {Component, DebugElement} from '@angular/core';
import {Component, DebugElement, ViewChild} from '@angular/core';
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {MAT_RIPPLE_GLOBAL_OPTIONS, RippleGlobalOptions} from '@angular/material/core';
import {By} from '@angular/platform-browser';
import {Subject} from 'rxjs';
import {MatChip, MatChipEvent, MatChipSelectionChange, MatChipsModule} from './index';
import {MatChip, MatChipEvent, MatChipSelectionChange, MatChipsModule, MatChipList} from './index';


describe('Chips', () => {
Expand Down Expand Up @@ -257,14 +257,27 @@ describe('Chips', () => {
expect(testComponent.chipSelectionChange).toHaveBeenCalledWith(CHIP_DESELECTED_EVENT);
});

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

testComponent.selected = true;
fixture.detectChanges();

expect(chipNativeElement.getAttribute('aria-selected')).toBe('true');
});

it('should have the correct aria-selected in multi-selection mode', () => {
testComponent.chipList.multiple = true;
fixture.detectChanges();

expect(chipNativeElement.getAttribute('aria-selected')).toBe('false');

testComponent.selected = true;
fixture.detectChanges();

expect(chipNativeElement.getAttribute('aria-selected')).toBe('true');
});

});

describe('when selectable is false', () => {
Expand Down Expand Up @@ -390,6 +403,7 @@ describe('Chips', () => {
</mat-chip-list>`
})
class SingleChip {
@ViewChild(MatChipList) chipList: MatChipList;
disabled: boolean = false;
name: string = 'Test';
color: string = 'primary';
Expand Down
8 changes: 7 additions & 1 deletion src/lib/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
/** Whether the chip list is selectable */
chipListSelectable: boolean = true;

/** Whether the chip list is in multi-selection mode. */
_chipListMultiple: boolean = false;

/** The chip avatar */
@ContentChild(MatChipAvatar) avatar: MatChipAvatar;

Expand Down Expand Up @@ -218,7 +221,10 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes

/** The ARIA selected applied to the chip. */
get ariaSelected(): string | null {
return this.selectable ? this.selected.toString() : 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;
}

constructor(public _elementRef: ElementRef,
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/lib/chips.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export declare const _MatChipMixinBase: CanColorCtor & CanDisableRippleCtor & Ca
export declare const MAT_CHIPS_DEFAULT_OPTIONS: InjectionToken<MatChipsDefaultOptions>;

export declare class MatChip extends _MatChipMixinBase implements FocusableOption, OnDestroy, CanColor, CanDisable, CanDisableRipple, RippleTarget {
_chipListMultiple: boolean;
_elementRef: ElementRef;
_hasFocus: boolean;
readonly _onBlur: Subject<MatChipEvent>;
Expand Down