Skip to content

Commit

Permalink
feat(list): Add notifyAction adapter for action on list item. (#4144)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Introduced new adapter method `getAttributeForElementIndex` to determine if target list item has `href` attribute and removed `followHref` adapter API.
  • Loading branch information
abhiomkar committed Jan 25, 2019
1 parent eb8b8f6 commit 6ed35b1
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 80 deletions.
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.
`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;
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);
}
}

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',
]);
});

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()));
});

0 comments on commit 6ed35b1

Please sign in to comment.