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

fix input textarea search autofocus bug #1103

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Conversation

pingan1927
Copy link
Contributor

@pingan1927 pingan1927 commented Apr 7, 2017

First of all, thanks for your contribution! :-)

Please makes sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow antd's code convention.
  • Run npm run lint and fix those errors before submitting in order to keep consistent code style.
  • Rebase before creating a PR to keep commit history clear.
  • Add some descriptions and refer relative issues for you PR.

@mention-bot
Copy link

@pingan1927, thanks for your PR! By analyzing the history of the files in this pull request, we identified @yiminghe, @afc163 and @benjycui to be potential reviewers.

@pingan1927 pingan1927 requested a review from paranoidjk April 7, 2017 07:33
}, 0);
if (!('focused' in this.props)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

话说这种判断为什么要用 in ? 直接 this.props.focused 不行吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是否存在,与布尔值结果还是有区别的吧。这个if我记得是承玉大神加进来的,我很想删掉..感觉没有用

Copy link
Contributor

Choose a reason for hiding this comment

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

我们的文档写的 focused bool false, 当然是应该判断 true false,而不是判断存在

Copy link
Contributor Author

Choose a reason for hiding this comment

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

focused?: boolean; ?指的是可以没有,有的话是boolean类型

Copy link
Contributor

Choose a reason for hiding this comment

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

没有就是 undefined,同样是false, this.props.focused 的判断就够了

Copy link
Member

Choose a reason for hiding this comment

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

最终显示结果是一样的。

但是写成 if (!this.props.focused) 的话,<Foo focused={fasle} /> 也会去 setState,区别在这里。

Copy link
Contributor

Choose a reason for hiding this comment

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

在这里 <Foo focused={fasle} /> 去 setState 看起来是合理的吧

Copy link
Member

@yesmeck yesmeck Apr 7, 2017

Choose a reason for hiding this comment

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

对的,�我搞错了。

Copy link
Member

Choose a reason for hiding this comment

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

@ @,重新捋一下:

情况一:

  1. <Foo />;
  2. onBlur 后需要去更新 this.state.focus;

情况二(focused 受控):

  1. <Foo focused={this.state.focused} onBlur={this.handleBlur} onFocus={this.handleFocus} />;
  2. onBlur 的时候如果 setState 了, 在 componentWillRecevie 的时候还会去 setState 一次,会多一次 setState

Copy link
Contributor

Choose a reason for hiding this comment

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

还没看代码,但如果是你说的2受控模式的话,内部的onBlur是不应该setState的,应该全部从props接收再设置

@paranoidjk paranoidjk merged commit 0797c3a into master Apr 7, 2017
@paranoidjk paranoidjk deleted the optimize-autofocus branch April 7, 2017 08:45
@silentcloud silentcloud mentioned this pull request Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants