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 @@ -504,11 +504,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

### `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 @@ -113,6 +107,11 @@ class MDCListAdapter {
* @param {boolean} isChecked
*/
setCheckedCheckboxOrRadioAtIndex(index, isChecked) {}

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

export default MDCListAdapter;
1 change: 1 addition & 0 deletions packages/mdc-list/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,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',
};

export {strings, cssClasses};
10 changes: 7 additions & 3 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: () => {},
});
}

Expand Down Expand Up @@ -249,6 +249,9 @@ class MDCListFoundation extends MDCFoundation {
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

if (this.isSingleSelectionList_) {
// Check if the space key was pressed on the list item or a child element.
this.preventDefaultEvent_(evt);
Expand All @@ -264,8 +267,7 @@ class MDCListFoundation extends MDCFoundation {
this.setSelectedIndex(currentIndex);
}

// 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 @@ -285,6 +287,8 @@ class MDCListFoundation extends MDCFoundation {
if (this.isSingleSelectionList_ || this.hasCheckboxOrRadioAtIndex_(index)) {
this.setSelectedIndex(index);
}

this.adapter_.notifyAction(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 @@ -241,12 +241,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 @@ -269,6 +263,9 @@ class MDCList extends MDCComponent {
event.initEvent('change', true, true);
toggleEl.dispatchEvent(event);
},
notifyAction: (index) => {
this.emit(strings.ACTION_EVENT, index, /** shouldBubble */ true);
},
})));
}
}
Expand Down
51 changes: 41 additions & 10 deletions test/unit/mdc-list/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCListFoundation, [
'getListItemCount', 'getFocusedElementIndex', 'setAttributeForElementIndex',
'removeAttributeForElementIndex', 'addClassForElementIndex', 'removeClassForElementIndex',
'focusItemAtIndex', 'setTabIndexForListItemChildren', 'followHref', 'hasRadioAtIndex',
'hasCheckboxAtIndex', 'isCheckboxCheckedAtIndex', 'setCheckedCheckboxOrRadioAtIndex',
'focusItemAtIndex', 'setTabIndexForListItemChildren', 'hasRadioAtIndex',
'hasCheckboxAtIndex', 'isCheckboxCheckedAtIndex', 'setCheckedCheckboxOrRadioAtIndex', 'notifyAction',
]);
});

Expand Down Expand Up @@ -425,25 +425,34 @@ test('#handleKeydown space/enter key does not cause event.preventDefault when si
td.verify(preventDefault(), {times: 0});
});

test('#handleKeydown space/enter key call adapter.followHref regardless of singleSelection', () => {
test('#handleKeydown space key calls notifyAction for anchor element regardless of singleSelection', () => {
const {foundation, mockAdapter} = setupTest();
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);
foundation.setSingleSelection(true);
foundation.handleKeydown(event, true, 0);
event.key = 'Space';
foundation.handleKeydown(event, true, 0);

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

test('#handleKeydown enter key does not call notifyAction for anchor element', () => {
const {foundation, mockAdapter} = setupTest();
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);
foundation.setSingleSelection(false);
foundation.handleKeydown(event, true, 0);
foundation.setSingleSelection(true);
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', () => {
Expand Down Expand Up @@ -472,6 +481,19 @@ test('#handleKeydown enter key does not cause preventDefault to be called if sin
td.verify(preventDefault(), {times: 0});
});

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']};
const event = {key: 'Enter', target, preventDefault};

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

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

test('#handleKeydown space key is triggered when singleSelection is true selects the list item', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
Expand Down Expand Up @@ -592,6 +614,15 @@ test('#handleClick when singleSelection=false on a list item should not cause th
td.verify(mockAdapter.setAttributeForElementIndex(1, 'tabindex', 0), {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
36 changes: 13 additions & 23 deletions test/unit/mdc-list/mdc-list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {assert} from 'chai';
import td from 'testdouble';
import bel from 'bel';
import {MDCList, MDCListFoundation} from '../../../packages/mdc-list/index';
import {strings} from '../../../packages/mdc-list/constants';

function getFixture() {
return bel`
Expand Down Expand Up @@ -234,29 +235,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 @@ -434,3 +412,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()));
});