From 6ed35b1203fc07895a9861f9461198653970482e Mon Sep 17 00:00:00 2001 From: Abhinay Omkar Date: Fri, 25 Jan 2019 16:21:27 -0500 Subject: [PATCH] feat(list): Add notifyAction adapter for action on list item. (#4144) BREAKING CHANGE: Introduced new adapter method `getAttributeForElementIndex` to determine if target list item has `href` attribute and removed `followHref` adapter API. --- packages/mdc-list/README.md | 2 +- packages/mdc-list/adapter.js | 11 +++-- packages/mdc-list/constants.js | 1 + packages/mdc-list/foundation.js | 12 +++-- packages/mdc-list/index.js | 9 ++-- test/unit/mdc-list/foundation.test.js | 63 ++++++++++----------------- test/unit/mdc-list/mdc-list.test.js | 37 ++++++---------- 7 files changed, 55 insertions(+), 80 deletions(-) diff --git a/packages/mdc-list/README.md b/packages/mdc-list/README.md index c4b0bfe1760..8caf11a272a 100644 --- a/packages/mdc-list/README.md +++ b/packages/mdc-list/README.md @@ -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` diff --git a/packages/mdc-list/adapter.js b/packages/mdc-list/adapter.js index b64ce003c58..a6abc29aec2 100644 --- a/packages/mdc-list/adapter.js +++ b/packages/mdc-list/adapter.js @@ -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. @@ -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. */ diff --git a/packages/mdc-list/constants.js b/packages/mdc-list/constants.js index 3fce66270bd..03a3d01507b 100644 --- a/packages/mdc-list/constants.js +++ b/packages/mdc-list/constants.js @@ -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} */ diff --git a/packages/mdc-list/foundation.js b/packages/mdc-list/foundation.js index a6fbc6a939d..2f29dc156a2 100644 --- a/packages/mdc-list/foundation.js +++ b/packages/mdc-list/foundation.js @@ -53,11 +53,11 @@ class MDCListFoundation extends MDCFoundation { removeClassForElementIndex: () => {}, focusItemAtIndex: () => {}, setTabIndexForListItemChildren: () => {}, - followHref: () => {}, hasRadioAtIndex: () => {}, hasCheckboxAtIndex: () => {}, isCheckboxCheckedAtIndex: () => {}, setCheckedCheckboxOrRadioAtIndex: () => {}, + notifyAction: () => {}, isFocusInsideList: () => {}, }); } @@ -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); } } @@ -255,6 +257,8 @@ class MDCListFoundation extends MDCFoundation { this.setSelectedIndexOnAction_(index, toggleCheckbox); } + this.adapter_.notifyAction(index); + this.setTabindexAtIndex_(index); this.focusedItemIndex_ = index; } diff --git a/packages/mdc-list/index.js b/packages/mdc-list/index.js index 8fbc2e7a092..57637fe7cbf 100644 --- a/packages/mdc-list/index.js +++ b/packages/mdc-list/index.js @@ -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); @@ -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); }, diff --git a/test/unit/mdc-list/foundation.test.js b/test/unit/mdc-list/foundation.test.js index cbbba4196c1..78fe6c871e8 100644 --- a/test/unit/mdc-list/foundation.test.js +++ b/test/unit/mdc-list/foundation.test.js @@ -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', ]); }); @@ -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']}; @@ -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']}; @@ -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']}; @@ -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']}; @@ -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', () => { @@ -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(); diff --git a/test/unit/mdc-list/mdc-list.test.js b/test/unit/mdc-list/mdc-list.test.js index b8379d0d315..1b6fdac8064 100644 --- a/test/unit/mdc-list/mdc-list.test.js +++ b/test/unit/mdc-list/mdc-list.test.js @@ -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` @@ -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); @@ -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())); +});