-
Notifications
You must be signed in to change notification settings - Fork 73
add onScrollChange event, ref antd-mobile/#1447. #42
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
Conversation
src/MultiPickerMixin.tsx
Outdated
| import MultiPickerProps from './MultiPickerProps'; | ||
|
|
||
| export default function(ComposedComponent) { | ||
| export default function (ComposedComponent) { |
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.onValueChange!(value, i); | ||
| } | ||
|
|
||
| onScrollChange = (i, v) => { |
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.
和 onValueChange 合并一下吧,实现都一样,减小体积
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.
不太能合并。。。onValueChange这个事件目前的定义是onScrollComplete,会有setState等一系列操作,而onScrollChange是滚动过程中的事件
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.
我是说和上面这个代码 onValueChange = (i, v) => {,抽出一个公共方法
|
@zhang740 web 和 rn(注意 android) 都测一下,没问题就合了 |
|
@silentcloud 应该没什么问题了,RN部分没改,测了下没啥问题,web部分也测了下没啥问题。 |
|
picker 和 mutilpePicker的 mixin rn 都有用到,如果 rn 不打算支持 onScrollChange 的话那就没啥问题 |
|
@silentcloud RN后续有需求再说吧 |
No description provided.