From 18dc8047586deb15295c0ae5bdb2235f6d27854e Mon Sep 17 00:00:00 2001
From: Benjy Cui <benjytrys@gmail.com>
Date: Sat, 5 May 2018 11:54:40 +0800
Subject: [PATCH 1/3] fix: Menu[onClick] should not be triggered by propagated
 onClick event

---
 examples/antd.js |  6 +++---
 src/DOMWrap.jsx  |  2 ++
 src/MenuItem.jsx | 14 ++++++--------
 src/SubMenu.jsx  |  9 ++++++++-
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/examples/antd.js b/examples/antd.js
index 7b20e2cb..bba71251 100644
--- a/examples/antd.js
+++ b/examples/antd.js
@@ -6,9 +6,9 @@ import Menu, { SubMenu, Item as MenuItem, Divider } from 'rc-menu';
 import 'rc-menu/assets/index.less';
 import animate from 'css-animation';
 
-function handleSelect(info) {
+function handleClick(info) {
+  console.log(`clicked ${info.key}`);
   console.log(info);
-  console.log(`selected ${info.key}`);
 }
 
 const animation = {
@@ -77,7 +77,7 @@ const nestSubMenu = (<SubMenu title={<span>offset sub menu 2</span>} key="4" pop
 function onOpenChange(value) {
   console.log('onOpenChange', value);
 }
-const commonMenu = (<Menu onSelect={handleSelect} onOpenChange={onOpenChange}>
+const commonMenu = (<Menu onClick={handleClick} onOpenChange={onOpenChange}>
   <SubMenu title={<span>sub menu</span>} key="1">
     <MenuItem key="1-1">0-1</MenuItem>
     <MenuItem key="1-2">0-2</MenuItem>
diff --git a/src/DOMWrap.jsx b/src/DOMWrap.jsx
index 6027cf45..ec3688c7 100644
--- a/src/DOMWrap.jsx
+++ b/src/DOMWrap.jsx
@@ -22,6 +22,8 @@ export default class DOMWrap extends React.Component {
     delete props.tag;
     delete props.hiddenClassName;
     delete props.visible;
+    // Otherwise, the propagated click event will trigger another onClick
+    props.onClick = null;
     return <Tag {...props} />;
   }
 }
diff --git a/src/MenuItem.jsx b/src/MenuItem.jsx
index 3a19feb7..00555ed2 100644
--- a/src/MenuItem.jsx
+++ b/src/MenuItem.jsx
@@ -167,14 +167,12 @@ export class MenuItem extends React.Component {
       // <li><a role='menuitem'>Link</a></li> would be a good example
       delete attrs.role;
     }
-    let mouseEvent = {};
-    if (!props.disabled) {
-      mouseEvent = {
-        onClick: this.onClick,
-        onMouseLeave: this.onMouseLeave,
-        onMouseEnter: this.onMouseEnter,
-      };
-    }
+    // In case that onClick/onMouseLeave/onMouseEnter is passed down from owner
+    const mouseEvent = {
+      onClick: props.disabled ? null : this.onClick,
+      onMouseLeave: props.disabled ? null : this.onMouseLeave,
+      onMouseEnter: props.disabled ? null : this.onMouseEnter,
+    };
     const style = {
       ...props.style,
     };
diff --git a/src/SubMenu.jsx b/src/SubMenu.jsx
index 1fce204c..d91aa416 100644
--- a/src/SubMenu.jsx
+++ b/src/SubMenu.jsx
@@ -491,8 +491,15 @@ export class SubMenu extends React.Component {
       subMenuCloseDelay,
     } = props;
     menuAllProps.forEach(key => delete props[key]);
+    // Set onClick to null, to ignore propagated onClick event
     return (
-      <li {...props} {...mouseEvents} className={className} role="menuitem">
+      <li
+        {...props}
+        {...mouseEvents}
+        className={className}
+        role="menuitem"
+        onClick={null}
+      >
         {isInlineMode && title}
         {isInlineMode && children}
         {!isInlineMode && (

From 0bc6b57fa3fd6c4d92cb9395af7cec2c4683b2ef Mon Sep 17 00:00:00 2001
From: guifu <pikaleize@gmail.com>
Date: Mon, 7 May 2018 14:23:32 +0800
Subject: [PATCH 2/3] chore: fix test

---
 tests/MenuItem.spec.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/MenuItem.spec.js b/tests/MenuItem.spec.js
index a66e643c..1fb54e1e 100644
--- a/tests/MenuItem.spec.js
+++ b/tests/MenuItem.spec.js
@@ -106,10 +106,8 @@ describe('MenuItem', () => {
       expect(wrapper.render()).toMatchSnapshot();
       wrapper.find('MenuItem').at(0).simulate('click');
       expect(onClick).toHaveBeenCalledTimes(1);
-      wrapper.find('SubMenu').at(0).simulate('click');
-      expect(onClick).toHaveBeenCalledTimes(2);
       wrapper.find('MenuItemGroup').at(0).simulate('click');
-      expect(onClick).toHaveBeenCalledTimes(3);
+      expect(onClick).toHaveBeenCalledTimes(2);
     });
   });
 

From b24f77b91d37a82e069705b4cf212322dc3bba0a Mon Sep 17 00:00:00 2001
From: guifu <pikaleize@gmail.com>
Date: Mon, 7 May 2018 14:29:14 +0800
Subject: [PATCH 3/3] chore: refactor

---
 src/DOMWrap.jsx     | 2 --
 src/SubMenu.jsx     | 3 ++-
 src/SubPopupMenu.js | 3 +++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/DOMWrap.jsx b/src/DOMWrap.jsx
index ec3688c7..6027cf45 100644
--- a/src/DOMWrap.jsx
+++ b/src/DOMWrap.jsx
@@ -22,8 +22,6 @@ export default class DOMWrap extends React.Component {
     delete props.tag;
     delete props.hiddenClassName;
     delete props.visible;
-    // Otherwise, the propagated click event will trigger another onClick
-    props.onClick = null;
     return <Tag {...props} />;
   }
 }
diff --git a/src/SubMenu.jsx b/src/SubMenu.jsx
index d91aa416..1e2dbe9c 100644
--- a/src/SubMenu.jsx
+++ b/src/SubMenu.jsx
@@ -492,13 +492,14 @@ export class SubMenu extends React.Component {
     } = props;
     menuAllProps.forEach(key => delete props[key]);
     // Set onClick to null, to ignore propagated onClick event
+    delete props.onClick;
+
     return (
       <li
         {...props}
         {...mouseEvents}
         className={className}
         role="menuitem"
-        onClick={null}
       >
         {isInlineMode && title}
         {isInlineMode && children}
diff --git a/src/SubPopupMenu.js b/src/SubPopupMenu.js
index 0fec13ab..2d08f4c4 100644
--- a/src/SubPopupMenu.js
+++ b/src/SubPopupMenu.js
@@ -328,6 +328,9 @@ export class SubPopupMenu extends React.Component {
     }
     const { prefixCls, eventKey, visible } = props;
     menuAllProps.forEach(key => delete props[key]);
+
+    // Otherwise, the propagated click event will trigger another onClick
+    delete props.onClick;
     return (
       // ESLint is not smart enough to know that the type of `children` was checked.
       /* eslint-disable */