Skip to content
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.

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

onChange callback will also pass option param #157

Closed
wants to merge 3 commits into from

Conversation

RaoHai
Copy link
Contributor

@RaoHai RaoHai commented Jan 17, 2017

afc163 referenced this pull request in ant-design/ant-design Jan 17, 2017
* cherry-pick

* remove useless files

* remove useless files

* Group ts

* update interface

* ReactElement
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 17a3bd3 on RaoHai:onChangeOption into ** on react-component:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c2fcfbb on RaoHai:onChangeOption into ** on react-component:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c2fcfbb on RaoHai:onChangeOption into ** on react-component:master**.

@RaoHai
Copy link
Contributor Author

RaoHai commented Jan 19, 2017

@yiminghe @yesmeck review 下。。

@@ -89,7 +89,7 @@ React.render(c, container);
|defaultValue | initial selected option(s) | String/Array<String> | - |
|value | current selected option(s) | String/Array<String>/{key:String, label:React.Node}/Array<{key, label}> | - |
|labelInValue| whether to embed label in value, see above value type | Bool | false |
|onChange | called when select an option or input value change(combobox) | function(value) | - |
|onChange | called when select an option or input value change(combobox) | function(value, option: Option | Array<Option>) | - |
Copy link
Member

@yesmeck yesmeck Jan 19, 2017

Choose a reason for hiding this comment

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

这个 Option 是指 <Option /> 还是 { key: '1', label: '1' }?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{ key: '1', label: '1' }

Copy link
Member

@yesmeck yesmeck Jan 20, 2017

Choose a reason for hiding this comment

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

issue 里说的应该是 <Option /> 吧, 他需要获取 props。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我看一下 onSelect。。。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

确实应该是 <Option />

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e76df9f on RaoHai:onChangeOption into ** on react-component:master**.

@RaoHai
Copy link
Contributor Author

RaoHai commented Jan 20, 2017

改好了。

  • 现在所有模式都有这个参数,multiple 的时候是数组 Array< <Option/> >, 其他时候是 <Option />

@@ -40,7 +40,7 @@ describe('Select.multiple', () => {
value: 'One,Two,Three',
} });

expect(handleChange).toBeCalledWith(['1', '2']);
expect(handleChange).toBeCalledWith(['1', '2'], [null, null]);
Copy link
Member

Choose a reason for hiding this comment

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

这里不能匹配到 Option 吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个例子有点怪。你自己试试,真实输入 One 是不会出来选项的,要输入 1 才行。但是 test 是通过的,

src/util.js Outdated
@@ -135,3 +135,13 @@ export function splitBySeparators(string, separators) {
}
return array;
}

export function findChildByKey(children, key) {
Copy link
Member

Choose a reason for hiding this comment

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

要考虑 OptGroup 的情况。

@yiminghe yiminghe assigned yesmeck and unassigned yiminghe Feb 15, 2017
@yesmeck
Copy link
Member

yesmeck commented Feb 16, 2017

@RaoHai

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b195636 on RaoHai:onChangeOption into ** on react-component:master**.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.533% when pulling 9239519 on RaoHai:onChangeOption into 3d7bc35 on react-component:master.

@RaoHai
Copy link
Contributor Author

RaoHai commented Feb 17, 2017

ci failed...

rc-tools run guard
/home/travis/build/react-component/select/node_modules/rc-tools/lib/gulpfile.js:31
const { tsCompiledDir } = require('./constants');
      ^
SyntaxError: Unexpected token {
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:413:25)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/home/travis/build/react-component/select/node_modules/rc-tools/lib/cli/run.js:31:3)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017 via email

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

rebase 下。

@RaoHai
Copy link
Contributor Author

RaoHai commented Feb 17, 2017

为什么出现了两个我。。

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

@ @ 还能这样。。。。

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.533% when pulling 5bdc17c on RaoHai:onChangeOption into 2e4b73e on react-component:master.

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

你一定用了两个不同的 git user 设置。

@RaoHai
Copy link
Contributor Author

RaoHai commented Feb 17, 2017

只是换了一台电脑而已。。

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

两台电脑上的 git config --get user.email 不一样吧

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

哈哈,是 git config --get user.name 不一样。

@yesmeck
Copy link
Member

yesmeck commented Feb 17, 2017

dingtalk20170217214701

@afc163
Copy link
Member

afc163 commented Feb 17, 2017

看东西重影。

@afc163 afc163 assigned yesmeck and unassigned yesmeck Feb 27, 2017
export function findChildBy(optionLabelProp, children, key) {
let child = null;
React.Children.forEach(children, child_ => {
if (child_.type === OptGroup) {
Copy link
Member

Choose a reason for hiding this comment

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

child_.type.isSelectOptGroup 判断。

React.Children.forEach(children, child_ => {
if (child_.type === OptGroup) {
child = child || findChildBy(optionLabelProp, child_.props.children, key);
} else if (child_.type === Option && (child_.key === key || child_.props.value === key)) {
Copy link
Member

Choose a reason for hiding this comment

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

child_.type.isSelectOption 判断

@@ -38,7 +38,7 @@ describe('Select.combobox', () => {

wrapper.find('input').simulate('change', { target: { value: '1' } });

expect(handleChange).toBeCalledWith('1');
expect(handleChange).toBeCalledWith('1', null);
Copy link
Member

Choose a reason for hiding this comment

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

没有 <Option /> 的时候,第二个参数是 null 还是 undefined?我看下面是 undefined

@yutingzhao1991
Copy link
Contributor

support in #264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

建议 Select 选择器的 onChange 函数增加 option 参数方便回调
6 participants