Skip to content

FIx InputNumber displays min value when min is > 0 and value is null #138

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

Jump to bottom
Merged
merged 4 commits into from
Jan 16, 2019
Merged

FIx InputNumber displays min value when min is > 0 and value is null #138

merged 4 commits into from
Jan 16, 2019

Conversation

@ztplz
Copy link
Contributor

@ztplz ztplz commented Jan 12, 2019

@ztplz
Copy link
Contributor Author

ztplz commented Jan 12, 2019

@afc163 @zombieJ
image
代码里很多情况都是用Number把一个参数转换 然后判断 但是这样有问题
image
这样也有问题

@coveralls
Copy link

coveralls commented Jan 12, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 87.543% when pulling a521018 on ztplz:min into 886503c on react-component:master.

@ztplz
Copy link
Contributor Author

ztplz commented Jan 12, 2019

image
我这么改也有问题 当value是0的时候

@zombieJ
Copy link
Member

zombieJ commented Jan 15, 2019

如果是数字的话，这边直接用 typeof nextValue === 'number' 好了。

@ztplz
Copy link
Contributor Author

ztplz commented Jan 15, 2019

我好像忘了 不知道是不是数字

@zombieJ
Copy link
Member

zombieJ commented Jan 15, 2019

我好像忘了 不知道是不是数字

不是数字按理大小判断应该都是 false，再加个测试把 string number object null undefined 都搞一遍就没啥问题了。

@ztplz
Copy link
Contributor Author

ztplz commented Jan 15, 2019

这个来源还有可能是props来的 可能是字符串 如果是字符串 就不会触发了

@ztplz
Copy link
Contributor Author

ztplz commented Jan 15, 2019

image

@zombieJ
Copy link
Member

zombieJ commented Jan 15, 2019

嗯，那测试用例就测测 string null undefined 变化吧

@ztplz
Copy link
Contributor Author

ztplz commented Jan 15, 2019

其实很多地方有这样的问题 这应该只是其中一个

@ztplz
Copy link
Contributor Author

ztplz commented Jan 16, 2019

@zombieJ Done.

@zombieJ zombieJ merged commit a521018 into react-component:master Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ztplz @coveralls @zombieJ