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 step floating point issue in NumberInput #1555

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

jqlio18
Copy link
Contributor

@jqlio18 jqlio18 commented Nov 20, 2022

Fixes #233, fixes #486, fixes #1554

Use the built-in function of steppers then assign the value.

JS has a bug with floating point which result in a visual bug where input has many decimals.
There's 2 ways to fix this :

  1. Converting float to an int (by multiplying by 10 x number of decimals) then make the equation then reconvert to a floating value.
  2. Use built-in functions to step up and down.

I prefer the second approach which is simpler.

Also, with this method we don't need to test the min and max values since stepUp and stepDown built-in functions do this internally. We do need to assign value to the input value because Svelte won't know that the value changed.

I also added a step test and updated the docs to add the step function.

For the margin I understand that because of the error Icon we can't reduce the padding, I'll update the issue.

Fixes carbon-design-system#1554: Use the built-in function of steppers then assign the value
@jqlio18 jqlio18 changed the title Fix floating point issue in NumberInput Fix step floating point issue in NumberInput Nov 20, 2022
@jqlio18 jqlio18 changed the title Fix step floating point issue in NumberInput Fix step floating point issue in NumberInput Nov 20, 2022
Copy link
Collaborator

@metonym metonym left a comment

Choose a reason for hiding this comment

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

This is brilliant.

@metonym metonym merged commit e6f5766 into carbon-design-system:master Dec 8, 2022
metonym added a commit that referenced this pull request Dec 8, 2022
Fast-follow to #1555. `ref.value` remains a string if not converted to a number.
@jqlio18 jqlio18 deleted the jqlio18/issue1554 branch December 8, 2022 04:09
@metonym
Copy link
Collaborator

metonym commented Dec 8, 2022

Thank you. Fixed in v0.70.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants