Skip to content

Commit

Permalink
fix(list): don't select disabled options when pressing ctrl + a (angu…
Browse files Browse the repository at this point in the history
…lar#18885)

Fixes disabled options being selected when the user presses ctrl + a. Selecting disabled options is still supported through `selectAll`.
  • Loading branch information
crisbeto authored Apr 20, 2020
1 parent 506b74a commit 77f65e3
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
47 changes: 47 additions & 0 deletions src/material/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ describe('MatSelectionList without forms', () => {
});

it('should select all items using ctrl + a', () => {
listOptions.forEach(option => option.componentInstance.disabled = false);
const event = createKeyboardEvent('keydown', A, selectionList.nativeElement);
Object.defineProperty(event, 'ctrlKey', {get: () => true});

Expand All @@ -505,6 +506,23 @@ describe('MatSelectionList without forms', () => {
expect(listOptions.every(option => option.componentInstance.selected)).toBe(true);
});

it('should not select disabled items when pressing ctrl + a', () => {
const event = createKeyboardEvent('keydown', A, selectionList.nativeElement);
Object.defineProperty(event, 'ctrlKey', {get: () => true});

listOptions.slice(0, 2).forEach(option => option.componentInstance.disabled = true);
fixture.detectChanges();

expect(listOptions.map(option => option.componentInstance.selected))
.toEqual([false, false, false, false, false]);

dispatchEvent(selectionList.nativeElement, event);
fixture.detectChanges();

expect(listOptions.map(option => option.componentInstance.selected))
.toEqual([false, false, true, true, true]);
});

it('should select all items using ctrl + a if some items are selected', () => {
const event = createKeyboardEvent('keydown', A, selectionList.nativeElement);
Object.defineProperty(event, 'ctrlKey', {get: () => true});
Expand Down Expand Up @@ -617,6 +635,20 @@ describe('MatSelectionList without forms', () => {
expect(list.options.toArray().every(option => option.selected)).toBe(true);
});

it('should be able to select all options, even if they are disabled', () => {
const list: MatSelectionList = selectionList.componentInstance;

list.options.forEach(option => option.disabled = true);
fixture.detectChanges();

expect(list.options.toArray().every(option => option.selected)).toBe(false);

list.selectAll();
fixture.detectChanges();

expect(list.options.toArray().every(option => option.selected)).toBe(true);
});

it('should be able to deselect all options', () => {
const list: MatSelectionList = selectionList.componentInstance;

Expand All @@ -629,6 +661,21 @@ describe('MatSelectionList without forms', () => {
expect(list.options.toArray().every(option => option.selected)).toBe(false);
});

it('should be able to deselect all options, even if they are disabled', () => {
const list: MatSelectionList = selectionList.componentInstance;

list.options.forEach(option => option.toggle());
expect(list.options.toArray().every(option => option.selected)).toBe(true);

list.options.forEach(option => option.disabled = true);
fixture.detectChanges();

list.deselectAll();
fixture.detectChanges();

expect(list.options.toArray().every(option => option.selected)).toBe(false);
});

it('should update the list value when an item is selected programmatically', () => {
const list: MatSelectionList = selectionList.componentInstance;

Expand Down
7 changes: 4 additions & 3 deletions src/material/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,8 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
// The "A" key gets special treatment, because it's used for the "select all" functionality.
if (keyCode === A && this.multiple && hasModifierKey(event, 'ctrlKey') &&
!manager.isTyping()) {
this.options.find(option => !option.selected) ? this.selectAll() : this.deselectAll();
const shouldSelect = this.options.some(option => !option.disabled && !option.selected);
this._setAllOptionsSelected(shouldSelect, true);
event.preventDefault();
} else {
manager.onKeydown(event);
Expand Down Expand Up @@ -663,13 +664,13 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
* Sets the selected state on all of the options
* and emits an event if anything changed.
*/
private _setAllOptionsSelected(isSelected: boolean) {
private _setAllOptionsSelected(isSelected: boolean, skipDisabled?: boolean) {
// Keep track of whether anything changed, because we only want to
// emit the changed event when something actually changed.
let hasChanged = false;

this.options.forEach(option => {
if (option._setSelected(isSelected)) {
if ((!skipDisabled || !option.disabled) && option._setSelected(isSelected)) {
hasChanged = true;
}
});
Expand Down

0 comments on commit 77f65e3

Please sign in to comment.