feat(menu): Fix menu to only fire one event per interaction

BREAKING CHANGE: Adds an adapter method eventTargetHasClass to check if a given event target has a given class

Author: amsheehan

packages/mdc-menu/README.md

@@ -269,6 +269,7 @@ The adapter for simple menu must provide the following functions, with correct s
 | `hasClass(className: string) => boolean` | Returns boolean indicating whether element has a given class. |
 | `hasNecessaryDom() => boolean` | Returns boolean indicating whether the necessary DOM is present (namely, the `mdc-simple-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`). |

packages/mdc-menu/simple/adapter.js

@@ -59,6 +59,13 @@ class MDCSimpleMenuAdapter {
    */
   getAttributeForEventTarget(target, attributeName) {}
 
+  /**
+   * @param {EventTarget} target
+   * @param {string} className
+   * @return {boolean}
+   */
+  eventTargetHasClass(target, className) {}
+
   /** @return {{ width: number, height: number }} */
   getInnerDimensions() {}
 

packages/mdc-menu/simple/constants.js

@@ -23,6 +23,7 @@ const cssClasses = {
   TOP_RIGHT: 'mdc-simple-menu--open-from-top-right',
   BOTTOM_LEFT: 'mdc-simple-menu--open-from-bottom-left',
   BOTTOM_RIGHT: 'mdc-simple-menu--open-from-bottom-right',
+  LIST_ITEM: 'mdc-list-item',
 };
 
 /** @enum {string} */

packages/mdc-menu/simple/foundation.js

@@ -51,6 +51,7 @@ class MDCSimpleMenuFoundation extends MDCFoundation {
       hasClass: () => false,
       hasNecessaryDom: () => false,
       getAttributeForEventTarget: () => {},
+      eventTargetHasClass: () => {},
       getInnerDimensions: () => ({}),
       hasAnchor: () => false,
       getAnchorDimensions: () => ({}),
@@ -91,10 +92,7 @@ class MDCSimpleMenuFoundation extends MDCFoundation {
     /** @private {function(!Event)} */
     this.keyupHandler_ = (evt) => this.handleKeyboardUp_(evt);
     /** @private {function(!Event)} */
-    this.documentClickHandler_ = (evt) => {
-      this.adapter_.notifyCancel();
-      this.close(evt);
-    };
+    this.documentClickHandler_ = (evt) => this.handleDocumentClick_(evt);
     /** @private {boolean} */
     this.isOpen_ = false;
     /** @private {number} */
@@ -275,6 +273,25 @@ class MDCSimpleMenuFoundation extends MDCFoundation {
     }
   }
 
+  /**
+   * Handle clicks and cancel the menu if not a 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)) {
+        return;
+      }
+      el = el.parentNode;
+    }
+
+    this.adapter_.notifyCancel();
+    this.close(evt);
+  };
+
   /**
    * Handle keys that we want to repeat on hold (tab and arrows).
    * @param {!Event} evt

packages/mdc-menu/simple/index.js

@@ -88,6 +88,7 @@ class MDCSimpleMenu 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};

test/unit/mdc-menu/mdc-simple-menu.test.js

@@ -37,8 +37,9 @@ function getFixture(open) {
 
 function setupTest(open = false) {
   const root = getFixture(open);
+  const listItem = root.querySelector('.mdc-list-item');
   const component = new MDCSimpleMenu(root);
-  return {root, component};
+  return {root, listItem, component};
 }
 
 suite('MDCSimpleMenu');
@@ -121,6 +122,13 @@ 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);

test/unit/mdc-menu/simple.foundation.test.js

@@ -62,12 +62,13 @@ test('exports numbers', () => {
 
 test('defaultAdapter returns a complete adapter implementation', () => {
   verifyDefaultAdapter(MDCSimpleMenuFoundation, [
-    'addClass', 'removeClass', 'hasClass', 'hasNecessaryDom', 'getAttributeForEventTarget', 'getInnerDimensions',
-    'hasAnchor', 'getAnchorDimensions', 'getWindowDimensions', 'setScale', 'setInnerScale', 'getNumberOfItems',
-    'registerInteractionHandler', 'deregisterInteractionHandler', 'registerBodyClickHandler',
-    'deregisterBodyClickHandler', 'getYParamsForItemAtIndex', 'setTransitionDelayForItemAtIndex',
-    'getIndexForEventTarget', 'notifySelected', 'notifyCancel', 'saveFocus', 'restoreFocus', 'isFocused', 'focus',
-    'getFocusedItemIndex', 'focusItemAtIndex', 'isRtl', 'setTransformOrigin', 'setPosition', 'getAccurateTime',
+    'addClass', 'removeClass', 'hasClass', 'hasNecessaryDom', 'getAttributeForEventTarget',
+    'eventTargetHasClass', 'getInnerDimensions', 'hasAnchor', 'getAnchorDimensions', 'getWindowDimensions',
+    'setScale', 'setInnerScale', 'getNumberOfItems', 'registerInteractionHandler', 'deregisterInteractionHandler',
+    'registerBodyClickHandler', 'deregisterBodyClickHandler', 'getYParamsForItemAtIndex',
+    'setTransitionDelayForItemAtIndex', 'getIndexForEventTarget', 'notifySelected', 'notifyCancel', 'saveFocus',
+    'restoreFocus', 'isFocused', 'focus', 'getFocusedItemIndex', 'focusItemAtIndex', 'isRtl', 'setTransformOrigin',
+    'setPosition', 'getAccurateTime',
   ]);
 });
 
@@ -837,17 +838,26 @@ test('on spacebar keydown prevents default on the event', () => {
   clock.uninstall();
 });
 
-testFoundation('on document click cancels and closes the menu', ({foundation, mockAdapter, mockRaf}) => {
+test('on document click cancels and closes the menu', () => {
+  const {foundation, mockAdapter} = setupTest();
+  const mockRaf = createMockRaf();
+  const mockEvt = {
+    target: {},
+  };
   let documentClickHandler;
   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.hasClass(MDCSimpleMenuFoundation.cssClasses.OPEN)).thenReturn(true);
 
   foundation.init();
   foundation.open();
   mockRaf.flush();
 
-  documentClickHandler();
+  documentClickHandler(mockEvt);
   mockRaf.flush();
 
   td.verify(mockAdapter.removeClass(cssClasses.OPEN));
@@ -856,6 +866,31 @@ testFoundation('on document click cancels and closes the menu', ({foundation, mo
   mockRaf.restore();
 });
 
+test('on menu item click does not emit cancel', () => {
+  const {foundation, mockAdapter} = setupTest();
+  const mockRaf = createMockRaf();
+  const mockEvt = {
+    target: {},
+  };
+  let documentClickHandler;
+  td.when(mockAdapter.registerBodyClickHandler(td.matchers.isA(Function))).thenDo((handler) => {
+    documentClickHandler = handler;
+  });
+  td.when(mockAdapter.eventTargetHasClass(td.matchers.anything(), cssClasses.LIST_ITEM))
+    .thenReturn(true);
+
+  foundation.init();
+  foundation.open();
+  mockRaf.flush();
+
+  documentClickHandler(mockEvt);
+  mockRaf.flush();
+
+  td.verify(mockAdapter.notifyCancel(), {times: 0});
+
+  mockRaf.restore();
+});
+
 testFoundation('should cancel animation after destroy', ({foundation, mockAdapter, mockRaf}) => {
   foundation.init();
   mockRaf.flush();