Skip to content
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

feat(list): Add notifyAction adapter for action on list item. #4144

Merged
merged 10 commits into from
Jan 25, 2019
2 changes: 1 addition & 1 deletion packages/mdc-list/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -506,11 +506,11 @@ Method Signature | Description
`removeClassForElementIndex(index: Number, className: String) => void` | Removes the `className` class to the list item at `index`.
`focusItemAtIndex(index: Number) => void` | Focuses the list item at the `index` value specified.
`setTabIndexForListItemChildren(index: Number, value: Number) => void` | Sets the `tabindex` attribute to `value` for each child button or anchor element in the list item at the `index` specified.
`followHref(element: Element) => void` | If the given element has an href, follows the link.
`hasRadioAtIndex(index: number) => boolean` | Returns true if radio button is present at given list item index.
`hasCheckboxAtIndex(index: number) => boolean` | Returns true if checkbox is present at given list item index.
`isCheckboxCheckedAtIndex(index: number) => boolean` | Returns true if checkbox inside a list item is checked.
`setCheckedCheckboxOrRadioAtIndex(index: number, isChecked: boolean) => void` | Sets the checked status of checkbox or radio at given list item index.
`notifyAction(index: number) => void` | Notifies user action on list item including keyboard and mouse actions.
abhiomkar marked this conversation as resolved.
Show resolved Hide resolved
`isFocusInsideList() => boolean` | Returns true if the current focused element is inside list root.

### `MDCListFoundation`
Expand Down
11 changes: 5 additions & 6 deletions packages/mdc-list/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@ class MDCListAdapter {
*/
setTabIndexForListItemChildren(listItemIndex, tabIndexValue) {}

/**
* If the given element has an href, follows the link.
* @param {!Element} ele
*/
followHref(ele) {}

/**
* @param {number} index
* @return {boolean} Returns true if radio button is present at given list item index.
Expand All @@ -114,6 +108,11 @@ class MDCListAdapter {
*/
setCheckedCheckboxOrRadioAtIndex(index, isChecked) {}

/**
* Notifies user action on list item.
*/
notifyAction(index) {}

/**
* @return {boolean} Returns true when the current focused element is inside list root.
*/
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-list/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const strings = {
.${cssClasses.LIST_ITEM_CLASS} input[type="radio"]:not(:disabled),
.${cssClasses.LIST_ITEM_CLASS} input[type="checkbox"]:not(:disabled)`,
ENABLED_ITEMS_SELECTOR: '.mdc-list-item:not(.mdc-list-item--disabled)',
ACTION_EVENT: 'MDCList:action',
};

/** @typedef {number|!Array<number>} */
Expand Down
12 changes: 8 additions & 4 deletions packages/mdc-list/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ class MDCListFoundation extends MDCFoundation {
removeClassForElementIndex: () => {},
focusItemAtIndex: () => {},
setTabIndexForListItemChildren: () => {},
followHref: () => {},
hasRadioAtIndex: () => {},
hasCheckboxAtIndex: () => {},
isCheckboxCheckedAtIndex: () => {},
setCheckedCheckboxOrRadioAtIndex: () => {},
notifyAction: () => {},
isFocusInsideList: () => {},
});
}
Expand Down Expand Up @@ -225,13 +225,15 @@ class MDCListFoundation extends MDCFoundation {
nextIndex = this.focusLastElement();
} else if (isEnter || isSpace) {
if (isRootListItem) {
// Return early if enter key is pressed on anchor element which triggers synthetic MouseEvent event.
if (evt.target.tagName === 'A' && isEnter) return;
abhiomkar marked this conversation as resolved.
Show resolved Hide resolved
this.preventDefaultEvent_(evt);

if (this.isSelectableList_()) {
this.setSelectedIndexOnAction_(currentIndex);
this.preventDefaultEvent_(evt);
}

// Explicitly activate links, since we're preventing default on Enter, and Space doesn't activate them.
this.adapter_.followHref(currentIndex);
this.adapter_.notifyAction(currentIndex);
kfranqueiro marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -255,6 +257,8 @@ class MDCListFoundation extends MDCFoundation {
this.setSelectedIndexOnAction_(index, toggleCheckbox);
}

this.adapter_.notifyAction(index);

this.setTabindexAtIndex_(index);
this.focusedItemIndex_ = index;
}
Expand Down
9 changes: 3 additions & 6 deletions packages/mdc-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,6 @@ class MDCList extends MDCComponent {
const listItemChildren = [].slice.call(element.querySelectorAll(strings.CHILD_ELEMENTS_TO_TOGGLE_TABINDEX));
listItemChildren.forEach((ele) => ele.setAttribute('tabindex', tabIndexValue));
},
followHref: (index) => {
const listItem = this.listElements[index];
if (listItem && listItem.href) {
listItem.click();
}
},
hasCheckboxAtIndex: (index) => {
const listItem = this.listElements[index];
return !!listItem.querySelector(strings.CHECKBOX_SELECTOR);
Expand All @@ -282,6 +276,9 @@ class MDCList extends MDCComponent {
event.initEvent('change', true, true);
toggleEl.dispatchEvent(event);
},
notifyAction: (index) => {
this.emit(strings.ACTION_EVENT, index, /** shouldBubble */ true);
},
isFocusInsideList: () => {
return this.root_.contains(document.activeElement);
},
Expand Down
63 changes: 24 additions & 39 deletions test/unit/mdc-list/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCListFoundation, [
'getListItemCount', 'getFocusedElementIndex', 'setAttributeForElementIndex',
'removeAttributeForElementIndex', 'addClassForElementIndex', 'removeClassForElementIndex',
'focusItemAtIndex', 'setTabIndexForListItemChildren', 'followHref', 'hasRadioAtIndex',
'focusItemAtIndex', 'setTabIndexForListItemChildren', 'hasRadioAtIndex',
'hasCheckboxAtIndex', 'isCheckboxCheckedAtIndex', 'setCheckedCheckboxOrRadioAtIndex',
'isFocusInsideList',
'notifyAction', 'isFocusInsideList',
abhiomkar marked this conversation as resolved.
Show resolved Hide resolved
]);
});

Expand Down Expand Up @@ -428,7 +428,7 @@ test('#handleKeydown focuses on the bound mdc-list-item even if the event happen
td.verify(preventDefault(), {times: 1});
});

test('#handleKeydown space key causes preventDefault to be called on the event when singleSelection=true', () => {
test('#handleKeydown space key causes preventDefault to be called on keydown event', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
Expand All @@ -442,7 +442,7 @@ test('#handleKeydown space key causes preventDefault to be called on the event w
td.verify(preventDefault(), {times: 1});
});

test('#handleKeydown enter key causes preventDefault to be called on the event when singleSelection=true', () => {
test('#handleKeydown enter key causes preventDefault to be called on keydown event', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
Expand Down Expand Up @@ -471,8 +471,7 @@ test('#handleKeydown enter key while focused on a sub-element of a list item doe
td.verify(preventDefault(), {times: 0});
});

test('#handleKeydown space/enter key cause event.preventDefault when singleSelection=false if a checkbox or ' +
'radio button is present', () => {
test('#handleKeydown space/enter key cause event.preventDefault if a checkbox or radio button is present', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
Expand All @@ -491,60 +490,37 @@ test('#handleKeydown space/enter key cause event.preventDefault when singleSelec
td.verify(preventDefault(), {times: 2});
});

test('#handleKeydown space/enter key does not cause event.preventDefault when singleSelection=false if a checkbox or ' +
'radio button is not present', () => {
test('#handleKeydown space key calls notifyAction for anchor element regardless of singleSelection', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
const event = {key: 'Enter', target, preventDefault};
const target = {tagName: 'A', classList: ['mdc-list-item']};
const event = {key: 'Space', target, preventDefault: () => {}};

td.when(mockAdapter.getFocusedElementIndex()).thenReturn(0);
td.when(mockAdapter.getListItemCount()).thenReturn(3);
td.when(mockAdapter.hasRadioAtIndex(0)).thenReturn(false);
td.when(mockAdapter.hasCheckboxAtIndex(0)).thenReturn(false);
foundation.setSingleSelection(false);
foundation.handleKeydown(event, true, 0);
event.key = 'Space';
foundation.setSingleSelection(true);
foundation.handleKeydown(event, true, 0);

td.verify(preventDefault(), {times: 0});
td.verify(mockAdapter.notifyAction(0), {times: 2});
});

test('#handleKeydown space/enter key call adapter.followHref regardless of singleSelection', () => {
test('#handleKeydown enter key does not call notifyAction for anchor element', () => {
const {foundation, mockAdapter} = setupTest();
const target = {classList: ['mdc-list-item']};
const target = {tagName: 'A', classList: ['mdc-list-item']};
const event = {key: 'Enter', target, preventDefault: () => {}};

td.when(mockAdapter.getFocusedElementIndex()).thenReturn(0);
td.when(mockAdapter.getListItemCount()).thenReturn(3);
td.when(mockAdapter.hasRadioAtIndex(0)).thenReturn(false);
td.when(mockAdapter.hasCheckboxAtIndex(0)).thenReturn(false);
foundation.setSingleSelection(false);
foundation.handleKeydown(event, true, 0);
foundation.setSingleSelection(true);
foundation.handleKeydown(event, true, 0);
event.key = 'Space';
foundation.handleKeydown(event, true, 0);
foundation.setSingleSelection(false);
foundation.handleKeydown(event, true, 0);

td.verify(mockAdapter.followHref(0), {times: 4});
td.verify(mockAdapter.notifyAction(0), {times: 0}); // notifyAction will be called by handleClick event.
});

test('#handleKeydown space key does not cause preventDefault to be called if singleSelection=false', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
const event = {key: 'Space', target, preventDefault};

td.when(mockAdapter.getFocusedElementIndex()).thenReturn(0);
td.when(mockAdapter.getListItemCount()).thenReturn(3);
foundation.handleKeydown(event, true, 0);

td.verify(preventDefault(), {times: 0});
});

test('#handleKeydown enter key does not cause preventDefault to be called if singleSelection=false', () => {
test('#handleKeydown notifies of action when enter key pressed on list item ', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
Expand All @@ -554,7 +530,7 @@ test('#handleKeydown enter key does not cause preventDefault to be called if sin
td.when(mockAdapter.getListItemCount()).thenReturn(3);
foundation.handleKeydown(event, true, 0);

td.verify(preventDefault(), {times: 0});
td.verify(mockAdapter.notifyAction(0), {times: 1});
});

test('#handleKeydown space key is triggered when singleSelection is true selects the list item', () => {
Expand Down Expand Up @@ -703,6 +679,15 @@ test('#handleClick when singleSelection=false on a list item should not cause th
td.verify(mockAdapter.addClassForElementIndex(1, cssClasses.LIST_ITEM_ACTIVATED_CLASS), {times: 0});
});

test('#handleClick notifies of action when clicked on list item.', () => {
const {foundation, mockAdapter} = setupTest();

td.when(mockAdapter.getListItemCount()).thenReturn(3);
foundation.handleClick(1, false);

td.verify(mockAdapter.notifyAction(1), {times: 1});
});

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

Expand Down
37 changes: 13 additions & 24 deletions test/unit/mdc-list/mdc-list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {assert} from 'chai';
import td from 'testdouble';
import bel from 'bel';
import {MDCList, MDCListFoundation} from '../../../packages/mdc-list/index';
import {cssClasses} from '../../../packages/mdc-list/constants';
import {cssClasses, strings} from '../../../packages/mdc-list/constants';

function getFixture() {
return bel`
Expand Down Expand Up @@ -261,29 +261,6 @@ test('adapter#setTabIndexForListItemChildren sets the child button/a elements of
document.body.removeChild(root);
});

test('adapter#followHref invokes click on element with href', () => {
const {root, component} = setupTest();
const anchorTag = document.createElement('a');
anchorTag.href = '#';
anchorTag.click = td.func('click');
anchorTag.classList.add('mdc-list-item');
root.appendChild(anchorTag);
component.getDefaultFoundation().adapter_.followHref(root.querySelectorAll('.mdc-list-item').length - 1);

td.verify(anchorTag.click(), {times: 1});
});

test('adapter#followHref does not invoke click on element without href', () => {
const {root, component} = setupTest();
const anchorTag = document.createElement('a');
anchorTag.click = td.func('click');
anchorTag.classList.add('mdc-list-item');
root.appendChild(anchorTag);
component.getDefaultFoundation().adapter_.followHref(root.querySelectorAll('.mdc-list-item').length - 1);

td.verify(anchorTag.click(), {times: 0});
});

test('layout adds tabindex=-1 to all list items without a tabindex', () => {
const {root} = setupTest();
assert.equal(0, root.querySelectorAll('.mdc-list-item:not([tabindex])').length);
Expand Down Expand Up @@ -468,3 +445,15 @@ test('adapter#setCheckedCheckboxOrRadioAtIndex toggles the radio on list item',
assert.isFalse(radio.checked);
document.body.removeChild(root);
});

test('adapter#notifyAction emits action event', () => {
const {component} = setupTest();

const handler = td.func('notifyActionHandler');

component.listen(strings.ACTION_EVENT, handler);
component.getDefaultFoundation().adapter_.notifyAction(3);
component.unlisten(strings.ACTION_EVENT, handler);

td.verify(handler(td.matchers.anything()));
});