Skip to content

[WIP]fix: passing onclick through #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[WIP]fix: passing onclick through #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions examples/antd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ function handleSelect(info) {
console.log(`selected ${info.key}`);
}

function handleItemClick(e) {
console.log('item clicked');
}

const animation = {
enter(node, done) {
let height;
Expand Down Expand Up @@ -52,41 +56,41 @@ const animation = {
const reactContainer = document.getElementById('__react-content');

const nestSubMenu = (<SubMenu title={<span>offset sub menu 2</span>} key="4" popupOffset={[10, 15]}>
<MenuItem key="4-1">inner inner</MenuItem>
<MenuItem key="4-1" whatever="inner inner" onClick={handleItemClick}>inner inner</MenuItem>
<Divider/>
<SubMenu
key="4-2"
title={<span>sub menu 3</span>}
>
<SubMenu title="sub 4-2-0" key="4-2-0">
<MenuItem key="4-2-0-1">inner inner</MenuItem>
<MenuItem key="4-2-0-2">inner inner2</MenuItem>
<MenuItem key="4-2-0-1" whatever="inner inner1" onClick={handleItemClick}>inner inner1</MenuItem>
<MenuItem key="4-2-0-2" whatever="inner inner2" onClick={handleItemClick}>inner inner2</MenuItem>
</SubMenu>
<MenuItem key="4-2-1">inn</MenuItem>
<MenuItem key="4-2-1" whatever="inner inner3" onClick={handleItemClick}>inner inner3</MenuItem>
<SubMenu title={<span>sub menu 4</span>} key="4-2-2">
<MenuItem key="4-2-2-1">inner inner</MenuItem>
<MenuItem key="4-2-2-2">inner inner2</MenuItem>
<MenuItem key="4-2-2-1" whatever="inner inner4" onClick={handleItemClick}>inner inner4</MenuItem>
<MenuItem key="4-2-2-2" whatever="inner inner5" onClick={handleItemClick}>inner inner5</MenuItem>
</SubMenu>
<SubMenu title="sub 4-2-3" key="4-2-3">
<MenuItem key="4-2-3-1">inner inner</MenuItem>
<MenuItem key="4-2-3-2">inner inner2</MenuItem>
<MenuItem key="4-2-3-1" whatever="inner inner6" onClick={handleItemClick}>inner inner6</MenuItem>
<MenuItem key="4-2-3-2" whatever="inner inner7" onClick={handleItemClick}>inner inner7</MenuItem>
</SubMenu>
</SubMenu>
</SubMenu>);

function onOpenChange(value) {
console.log('onOpenChange', value);
}
const commonMenu = (<Menu onSelect={handleSelect} onOpenChange={onOpenChange}>
const commonMenu = (<Menu onClick={handleSelect} onOpenChange={onOpenChange}>
<SubMenu title={<span>sub menu</span>} key="1">
<MenuItem key="1-1">0-1</MenuItem>
<MenuItem key="1-2">0-2</MenuItem>
<MenuItem key="1-1" onClick={handleItemClick}>0-1</MenuItem>
<MenuItem key="1-2" onClick={handleItemClick}>0-2</MenuItem>
</SubMenu>
{nestSubMenu}
<MenuItem key="2">1</MenuItem>
<MenuItem key="3">outer</MenuItem>
<MenuItem key="2" onClick={handleItemClick}>1</MenuItem>
<MenuItem key="3" onClick={handleItemClick}>outer</MenuItem>
<MenuItem disabled>disabled</MenuItem>
<MenuItem key="5">outer3</MenuItem>
<MenuItem key="5" onClick={handleItemClick}>outer3</MenuItem>
</Menu>);

function render(container) {
Expand Down
4 changes: 1 addition & 3 deletions src/SubMenu.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
noop,
loopMenuItemRecursively,
getMenuIdFromSubMenuEventKey,
menuAllProps,
} from './util';

let guid = 0;
Expand Down Expand Up @@ -490,9 +489,8 @@ export class SubMenu extends React.Component {
forceSubMenuRender,
subMenuCloseDelay,
} = props;
menuAllProps.forEach(key => delete props[key]);
return (
<li {...props} {...mouseEvents} className={className} role="menuitem">
Copy link
Contributor Author

@picodoth picodoth May 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jljsj33 亲 我们这个地方一定要{...props}么？因为现在这个li是包着children的，所以{...props}肯定会有问题。照道理说菜单title的onClick事件就应该是展开或者关闭子菜单啊？

<li {...mouseEvents} className={className} role="menuitem">
{isInlineMode && title}
{isInlineMode && children}
{!isInlineMode && (
Expand Down
5 changes: 2 additions & 3 deletions src/SubPopupMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { connect } from 'mini-store';
import KeyCode from 'rc-util/lib/KeyCode';
import createChainedFunction from 'rc-util/lib/createChainedFunction';
import classNames from 'classnames';
import { getKeyFromChildrenIndex, loopMenuItem, noop, menuAllProps } from './util';
import { getKeyFromChildrenIndex, loopMenuItem, noop } from './util';
import DOMWrap from './DOMWrap';

function allDisabled(arr) {
Expand Down Expand Up @@ -327,12 +327,11 @@ export class SubPopupMenu extends React.Component {
domProps.onKeyDown = this.onKeyDown;
}
const { prefixCls, eventKey, visible } = props;
menuAllProps.forEach(key => delete props[key]);
return (
// ESLint is not smart enough to know that the type of `children` was checked.
/* eslint-disable */
<DOMWrap
{...props}
Copy link
Contributor Author

@picodoth picodoth May 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个地方也是，感觉更加没有理由{...props}

Copy link
Member

@yesmeck yesmeck May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jljsj33 主要是这里

style={props.style}
tag="ul"
hiddenClassName={`${prefixCls}-hidden`}
visible={visible}
Expand Down
4 changes: 0 additions & 4 deletions tests/MenuItem.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ 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);
});
});

Expand Down
2 changes: 0 additions & 2 deletions tests/__snapshots__/MenuItem.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ exports[`MenuItem rest props can render all props to sub component 1`] = `
</li>
<li
class="rc-menu-submenu rc-menu-submenu-inline className"
data-whatever="whatever"
role="menuitem"
style="font-size: 20px;"
>
<div
aria-expanded="false"
Expand Down