Merge pull request #140 from react-component/fix-click

fix: Menu[onClick] should not be triggered by propagated onClick event

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>

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,
     };

src/SubMenu.jsx

@@ -491,8 +491,16 @@ export class SubMenu extends React.Component {
       subMenuCloseDelay,
     } = 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">
+      <li
+        {...props}
+        {...mouseEvents}
+        className={className}
+        role="menuitem"
+      >
         {isInlineMode && title}
         {isInlineMode && children}
         {!isInlineMode && (

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 */

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);
     });
   });