Skip to content

Commit 4cfdb20

Browse files
devversionmmalerba
authored andcommitted
fix(selection-list): do not allow toggling disabled options (#12617)
* fix(selection-list): do not allow toggling disabled options * Currently due to a misplaced `disabled` check in the selection list, users can toggle the state of a disabled `<mat-option>` by using the keyboard. Fixes #12608 * Rename function
1 parent ceb2051 commit 4cfdb20

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/lib/list/selection-list.spec.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,21 @@ describe('MatSelectionList without forms', () => {
211211
expect(ENTER_EVENT.defaultPrevented).toBe(true);
212212
});
213213

214+
it('should not be able to toggle a disabled option using SPACE', () => {
215+
const testListItem = listOptions[1].nativeElement as HTMLElement;
216+
const selectionModel = selectionList.componentInstance.selectedOptions;
217+
218+
expect(selectionModel.selected.length).toBe(0);
219+
220+
listOptions[1].componentInstance.disabled = true;
221+
222+
dispatchFakeEvent(testListItem, 'focus');
223+
selectionList.componentInstance._keydown(createKeyboardEvent('keydown', SPACE, testListItem));
224+
fixture.detectChanges();
225+
226+
expect(selectionModel.selected.length).toBe(0);
227+
});
228+
214229
it('should restore focus if active option is destroyed', () => {
215230
const manager = selectionList.componentInstance._keyManager;
216231

src/lib/list/selection-list.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -410,12 +410,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
410410
switch (keyCode) {
411411
case SPACE:
412412
case ENTER:
413-
if (!this.disabled) {
414-
this._toggleSelectOnFocusedOption();
415-
416-
// Always prevent space from scrolling the page since the list has focus
417-
event.preventDefault();
418-
}
413+
this._toggleFocusedOption();
414+
// Always prevent space from scrolling the page since the list has focus
415+
event.preventDefault();
419416
break;
420417
case HOME:
421418
case END:
@@ -434,7 +431,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
434431

435432
if ((keyCode === UP_ARROW || keyCode === DOWN_ARROW) && event.shiftKey &&
436433
manager.activeItemIndex !== previousFocusIndex) {
437-
this._toggleSelectOnFocusedOption();
434+
this._toggleFocusedOption();
438435
}
439436
}
440437

@@ -492,14 +489,14 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
492489
return this.options.filter(option => option.selected).map(option => option.value);
493490
}
494491

495-
/** Toggles the selected state of the currently focused option. */
496-
private _toggleSelectOnFocusedOption(): void {
492+
/** Toggles the state of the currently focused option if enabled. */
493+
private _toggleFocusedOption(): void {
497494
let focusedIndex = this._keyManager.activeItemIndex;
498495

499496
if (focusedIndex != null && this._isValidIndex(focusedIndex)) {
500497
let focusedOption: MatListOption = this.options.toArray()[focusedIndex];
501498

502-
if (focusedOption) {
499+
if (focusedOption && !focusedOption.disabled) {
503500
focusedOption.toggle();
504501

505502
// Emit a change event because the focused option changed its state through user

0 commit comments

Comments
 (0)