Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
fix(list): No longer emits action event when disabled item selected
Browse files Browse the repository at this point in the history
This should fix both menu and select bug where disabled items can be selected (closes #5571).

PiperOrigin-RevId: 297681923
  • Loading branch information
allan-chen authored and copybara-github committed Feb 27, 2020
1 parent 3ca8c4c commit f352d03
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 7 deletions.
18 changes: 11 additions & 7 deletions packages/mdc-list/foundation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ export class MDCListFoundation extends MDCFoundation<MDCListAdapter> {
}
this.preventDefaultEvent_(evt);

if (this.adapter_.listItemAtIndexHasClass(
currentIndex, cssClasses.LIST_ITEM_DISABLED_CLASS)) {
return;
}
if (this.isSelectableList_()) {
this.setSelectedIndexOnAction_(currentIndex);
}
Expand All @@ -247,14 +251,18 @@ export class MDCListFoundation extends MDCFoundation<MDCListAdapter> {
return;
}

this.setTabindexAtIndex_(index);
this.focusedItemIndex_ = index;

if (this.adapter_.listItemAtIndexHasClass(
index, cssClasses.LIST_ITEM_DISABLED_CLASS)) {
return;
}
if (this.isSelectableList_()) {
this.setSelectedIndexOnAction_(index, toggleCheckbox);
}

this.adapter_.notifyAction(index);

this.setTabindexAtIndex_(index);
this.focusedItemIndex_ = index;
}

/**
Expand Down Expand Up @@ -468,10 +476,6 @@ export class MDCListFoundation extends MDCFoundation<MDCListAdapter> {
* User interaction should not toggle list item(s) when disabled.
*/
private setSelectedIndexOnAction_(index: number, toggleCheckbox = true) {
if (this.adapter_.listItemAtIndexHasClass(index, cssClasses.LIST_ITEM_DISABLED_CLASS)) {
return;
}

if (this.isCheckboxList_) {
this.toggleCheckboxAtIndex_(index, toggleCheckbox);
} else {
Expand Down
29 changes: 29 additions & 0 deletions packages/mdc-list/test/foundation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,22 @@ describe('MDCListFoundation', () => {
expect(mockAdapter.notifyAction).toHaveBeenCalledTimes(2);
});

it('#handleKeydown space key does not call notifyAction for disabled element',
() => {
const {foundation, mockAdapter} = setupTest();
const target = {tagName: 'A', classList: ['mdc-list-item']};
const event = {key: 'Space', target, preventDefault: () => {}};

mockAdapter.getFocusedElementIndex.and.returnValue(0);
mockAdapter.getListItemCount.and.returnValue(3);
mockAdapter.listItemAtIndexHasClass
.withArgs(0, cssClasses.LIST_ITEM_DISABLED_CLASS)
.and.returnValue(true);
foundation.handleKeydown(event, true, 0);

expect(mockAdapter.notifyAction).not.toHaveBeenCalled();
});

it('#handleKeydown enter key does not call notifyAction for anchor element',
() => {
const {foundation, mockAdapter} = setupTest();
Expand Down Expand Up @@ -897,6 +913,19 @@ describe('MDCListFoundation', () => {
expect(mockAdapter.notifyAction).toHaveBeenCalledTimes(1);
});

it('#handleClick does not notify of action when clicked on disabled list item.',
() => {
const {foundation, mockAdapter} = setupTest();

mockAdapter.getListItemCount.and.returnValue(3);
mockAdapter.listItemAtIndexHasClass
.withArgs(1, cssClasses.LIST_ITEM_DISABLED_CLASS)
.and.returnValue(true);
foundation.handleClick(1, false);

expect(mockAdapter.notifyAction).not.toHaveBeenCalled();
});

it('#handleClick when singleSelection=true on a list item should cause the list item to be selected',
() => {
const {foundation, mockAdapter} = setupTest();
Expand Down

0 comments on commit f352d03

Please sign in to comment.