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-number add error #1008

Merged
merged 3 commits into from
Aug 31, 2021
Merged

fix: input-number add error #1008

merged 3 commits into from
Aug 31, 2021

Conversation

Talljack
Copy link
Contributor

close #1007

@vercel
Copy link

vercel bot commented Aug 29, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tusimple/naive-ui/6rRYbY5p1kDh15mpeTeG4maYXUiA
✅ Preview: https://naive-ui-git-fork-talljack-1007-tusimple.vercel.app

@codecov
Copy link

codecov bot commented Aug 29, 2021

Codecov Report

Merging #1008 (ad42c5a) into main (70fafa6) will increase coverage by 1.49%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1008      +/-   ##
==========================================
+ Coverage   45.57%   47.07%   +1.49%     
==========================================
  Files         509      511       +2     
  Lines       12478    12593     +115     
  Branches     3510     3548      +38     
==========================================
+ Hits         5687     5928     +241     
+ Misses       5789     5644     -145     
- Partials     1002     1021      +19     
Impacted Files Coverage Δ
src/input-number/src/InputNumber.tsx 61.41% <100.00%> (+8.89%) ⬆️
src/tree/src/utils.ts 14.28% <0.00%> (-2.39%) ⬇️
src/tree-select/src/TreeSelect.tsx 24.15% <0.00%> (-0.41%) ⬇️
src/image/src/Image.tsx 78.57% <0.00%> (ø)
src/tree/src/interface.ts 100.00% <0.00%> (ø)
src/upload/styles/light.ts 100.00% <0.00%> (ø)
src/upload/src/interface.ts 100.00% <0.00%> (ø)
src/tree-select/src/utils.ts 11.62% <0.00%> (ø)
src/tree/src/TreeNodeContent.tsx 62.50% <0.00%> (ø)
src/_internal/icons/Photo.tsx 100.00% <0.00%> (ø)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70fafa6...ad42c5a. Read the comment docs.

@@ -143,7 +143,7 @@ export default defineComponent({
return null
}
if (validator(parsedValue)) {
let nextValue = parsedValue + offset
let nextValue = parseFloat((parsedValue + offset).toFixed(10))
Copy link
Contributor

Choose a reason for hiding this comment

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

这么做是不准确的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我查了一下很多地方都是这个方法,你有啥更好的办法吗

Copy link
Contributor

Choose a reason for hiding this comment

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

@Talljack 我的思路是取parsedValue与offset值的精度最大值maxPrecision,然后Math.pow()进行精度的处理,最后toFixed(maxPrecision)这样......

Copy link
Collaborator

Choose a reason for hiding this comment

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

在 slider 里面有人处理了类似的问题,可以参考一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@07akioni
Copy link
Collaborator

@Talljack 测试安排

@07akioni
Copy link
Collaborator

这种能找到原本实现 bug 的 case 都是很适合写单元测试的

@@ -143,7 +150,7 @@ export default defineComponent({
return null
}
if (validator(parsedValue)) {
let nextValue = parsedValue + offset
let nextValue = parseFloat((parsedValue + offset).toFixed(precisionRef.value))
Copy link
Contributor

Choose a reason for hiding this comment

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

1.005.toFixed(2) 返回 "1.00",看起来 toFixed 不是一个安全的解决方案

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.005应该会取到最大的precisions,precisionRef.value会为3

@07akioni 07akioni merged commit aa47a1d into tusen-ai:main Aug 31, 2021
rhengles pushed a commit to arijs/naive-ui that referenced this pull request Oct 20, 2021
* fix: input-number add error

* fix: floating point decimal precision

* fix: add test

Co-authored-by: yugang.cao <yugang.cao@tusimple.ai>
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.

n-input-number
4 participants