Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
fix(menu): Close menu when a list-item was clicked. (#1756)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Removes the `eventTargetHasClass` from the adapter.

Previously clicks on a list-item in the page would leave the menu open.
Now we check if a item in this menu was clicked and leave the menu open.

Fixes #1747
  • Loading branch information
simono authored and williamernest committed Feb 2, 2018
1 parent d83d5bd commit c052cfe
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 27 deletions.
1 change: 0 additions & 1 deletion packages/mdc-menu/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ Method Signature | Description
`hasClass(className: string) => boolean` | Returns a boolean indicating whether the root element has a given class.
`hasNecessaryDom() => boolean` | Returns boolean indicating whether the necessary DOM is present (namely, the `mdc-menu__items` container).
`getAttributeForEventTarget(target: EventTarget, attributeName: string) => string` | Returns the value of a given attribute on an event target.
`eventTargetHasClass: (target: EventTarget, className: string) => boolean` | Returns true if the event target has a given class.
`getInnerDimensions() => {width: number, height: number}` | Returns an object with the items container width and height.
`hasAnchor: () => boolean` | Returns whether the menu has an anchor for positioning.
`getAnchorDimensions() => {width: number, height: number, top: number, right: number, bottom: number, left: number}` | Returns an object with the dimensions and position of the anchor (same semantics as `DOMRect`).
Expand Down
7 changes: 0 additions & 7 deletions packages/mdc-menu/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,6 @@ class MDCMenuAdapter {
*/
getAttributeForEventTarget(target, attributeName) {}

/**
* @param {EventTarget} target
* @param {string} className
* @return {boolean}
*/
eventTargetHasClass(target, className) {}

/** @return {{ width: number, height: number }} */
getInnerDimensions() {}

Expand Down
1 change: 0 additions & 1 deletion packages/mdc-menu/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const cssClasses = {
OPEN: 'mdc-menu--open',
ANIMATING_OPEN: 'mdc-menu--animating-open',
ANIMATING_CLOSED: 'mdc-menu--animating-closed',
LIST_ITEM: 'mdc-list-item',
SELECTED_LIST_ITEM: 'mdc-list-item--selected',
};

Expand Down
5 changes: 2 additions & 3 deletions packages/mdc-menu/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class MDCMenuFoundation extends MDCFoundation {
hasClass: () => false,
hasNecessaryDom: () => false,
getAttributeForEventTarget: () => {},
eventTargetHasClass: () => {},
getInnerDimensions: () => ({}),
hasAnchor: () => false,
getAnchorDimensions: () => ({}),
Expand Down Expand Up @@ -239,15 +238,15 @@ class MDCMenuFoundation extends MDCFoundation {
}

/**
* Handle clicks and cancel the menu if not a list item
* Handle clicks and cancel the menu if not a child list-item
* @param {!Event} evt
* @private
*/
handleDocumentClick_(evt) {
let el = evt.target;

while (el && el !== document.documentElement) {
if (this.adapter_.eventTargetHasClass(el, cssClasses.LIST_ITEM)) {
if (this.adapter_.getIndexForEventTarget(el) !== -1) {
return;
}
el = el.parentNode;
Expand Down
1 change: 0 additions & 1 deletion packages/mdc-menu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ class MDCMenu extends MDCComponent {
hasClass: (className) => this.root_.classList.contains(className),
hasNecessaryDom: () => Boolean(this.itemsContainer_),
getAttributeForEventTarget: (target, attributeName) => target.getAttribute(attributeName),
eventTargetHasClass: (target, className) => target.classList.contains(className),
getInnerDimensions: () => {
const {itemsContainer_: itemsContainer} = this;
return {width: itemsContainer.offsetWidth, height: itemsContainer.offsetHeight};
Expand Down
10 changes: 1 addition & 9 deletions test/unit/mdc-menu/mdc-simple-menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ function getFixture(open) {

function setupTest(open = false) {
const root = getFixture(open);
const listItem = root.querySelector('.mdc-list-item');
const component = new MDCMenu(root);
return {root, listItem, component};
return {root, component};
}

suite('MDCMenu');
Expand Down Expand Up @@ -151,13 +150,6 @@ test('adapter#getAttributeForEventTarget returns the value of an attribute for a
assert.equal(component.getDefaultFoundation().adapter_.getAttributeForEventTarget(target, attrName), attrVal);
});

test('adapter#eventTargetHasClass returns true if a target has a given classname', () => {
const {listItem, component} = setupTest();
listItem.classList.add('foo');

assert.isTrue(component.getDefaultFoundation().adapter_.eventTargetHasClass(listItem, 'foo'));
});

test('adapter#hasNecessaryDom returns false if the DOM does not include the items container', () => {
const {root, component} = setupTest();
const items = root.querySelector(strings.ITEMS_SELECTOR);
Expand Down
8 changes: 3 additions & 5 deletions test/unit/mdc-menu/simple.foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ test('exports numbers', () => {

test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCMenuFoundation, [
'addClass', 'removeClass', 'hasClass', 'hasNecessaryDom', 'getAttributeForEventTarget', 'eventTargetHasClass',
'addClass', 'removeClass', 'hasClass', 'hasNecessaryDom', 'getAttributeForEventTarget',
'getInnerDimensions', 'hasAnchor', 'getAnchorDimensions', 'getWindowDimensions',
'getNumberOfItems', 'registerInteractionHandler', 'deregisterInteractionHandler', 'registerBodyClickHandler',
'deregisterBodyClickHandler', 'getIndexForEventTarget', 'notifySelected', 'notifyCancel', 'saveFocus',
Expand Down Expand Up @@ -1058,8 +1058,7 @@ test('on document click cancels and closes the menu', () => {
td.when(mockAdapter.registerBodyClickHandler(td.matchers.isA(Function))).thenDo((handler) => {
documentClickHandler = handler;
});
td.when(mockAdapter.eventTargetHasClass(td.matchers.anything(), cssClasses.LIST_ITEM))
.thenReturn(false);
td.when(mockAdapter.getIndexForEventTarget(td.matchers.anything())).thenReturn(-1);

td.when(mockAdapter.hasClass(MDCMenuFoundation.cssClasses.OPEN)).thenReturn(true);

Expand All @@ -1086,8 +1085,7 @@ test('on menu item click does not emit cancel', () => {
td.when(mockAdapter.registerBodyClickHandler(td.matchers.isA(Function))).thenDo((handler) => {
documentClickHandler = handler;
});
td.when(mockAdapter.eventTargetHasClass(td.matchers.anything(), cssClasses.LIST_ITEM))
.thenReturn(true);
td.when(mockAdapter.getIndexForEventTarget(td.matchers.anything())).thenReturn(0);

foundation.init();
foundation.open();
Expand Down

0 comments on commit c052cfe

Please sign in to comment.