Skip to content

Commit

Permalink
fix(menu): Read index property from list item event detail (#4368)
Browse files Browse the repository at this point in the history
This fixes a regression caused by PR #4356, and adds a unit test to catch future integration failures between list and menu events.
  • Loading branch information
acdvorak committed Feb 7, 2019
1 parent 13b02b5 commit 5eb5a01
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
2 changes: 1 addition & 1 deletion packages/mdc-menu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class MDCMenu extends MDCComponent {
initialSyncWithDOM() {
this.afterOpenedCallback_ = () => this.handleAfterOpened_();
this.handleKeydown_ = (evt) => this.foundation_.handleKeydown(evt);
this.handleItemAction_ = (evt) => this.foundation_.handleItemAction(this.items[evt.detail]);
this.handleItemAction_ = (evt) => this.foundation_.handleItemAction(this.items[evt.detail.index]);

this.menuSurface_.listen(MDCMenuSurfaceFoundation.strings.OPENED_EVENT, this.afterOpenedCallback_);
this.listen('keydown', this.handleKeydown_);
Expand Down
32 changes: 30 additions & 2 deletions test/unit/mdc-menu/mdc-menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ class FakeMenuSurface {
}
}

/**
* @param {boolean=} open
* @return {{
* component: !MDCMenu,
* menuSurface: !MDCMenuSurface,
* root: !HTMLElement,
* list: !MDCList,
* mockFoundation: !MDCMenuFoundation
* }}
*/
function setupTestWithFakes(open = false) {
const root = getFixture(open);

Expand All @@ -89,6 +99,10 @@ function setupTestWithFakes(open = false) {
return {root, component, menuSurface, list, mockFoundation};
}

/**
* @param {boolean=} open
* @return {{component: !MDCMenu, root: !HTMLElement}}
*/
function setupTest(open = false) {
const root = getFixture(open);

Expand Down Expand Up @@ -121,7 +135,7 @@ test('attachTo initializes and returns a MDCMenu instance', () => {

test('initialize registers event listener for list item action', () => {
const {mockFoundation, root} = setupTestWithFakes();
domEvents.emit(root, MDCListFoundation.strings.ACTION_EVENT, {detail: 0});
domEvents.emit(root, MDCListFoundation.strings.ACTION_EVENT, {detail: {index: 0}});
td.verify(mockFoundation.handleItemAction(td.matchers.isA(Element)), {times: 1});
});

Expand All @@ -135,7 +149,7 @@ test('destroy deregisters event listener for click', () => {
const {component, mockFoundation, root} = setupTestWithFakes();
component.destroy();

domEvents.emit(root, MDCListFoundation.strings.ACTION_EVENT, {detail: 0});
domEvents.emit(root, MDCListFoundation.strings.ACTION_EVENT, {detail: {index: 0}});
td.verify(mockFoundation.handleItemAction(td.matchers.isA(Element)), {times: 0});
});

Expand Down Expand Up @@ -276,6 +290,20 @@ test('menu surface opened event causes no element to be focused if the list is e
document.body.removeChild(root);
});

test('list item enter keydown emits a menu action event', () => {
const {root, component} = setupTest();
const fakeEnterKeyEvent = {key: 'Enter', target: {tagName: 'div'}, preventDefault: () => undefined};

let detail = undefined;
component.listen(MDCMenuFoundation.strings.SELECTED_EVENT, (evt) => detail = evt.detail);

document.body.appendChild(root);
component.list_.foundation_.handleKeydown(fakeEnterKeyEvent, /* isRootListItem */ true, /* listItemIndex */ 0);
document.body.removeChild(root);

assert.deepEqual(detail, {index: 0, item: component.items[0]});
});

test('open=true does not throw an error if there are no items in the list to focus', () => {
const {component, root, list} = setupTestWithFakes();
list.listElements = [];
Expand Down

0 comments on commit 5eb5a01

Please sign in to comment.