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

Allow decimals to end in "zero" #433

Closed
2 tasks done
rodrigorahal opened this issue Dec 26, 2016 · 8 comments
Closed
2 tasks done

Allow decimals to end in "zero" #433

rodrigorahal opened this issue Dec 26, 2016 · 8 comments
Labels

Comments

@rodrigorahal
Copy link

rodrigorahal commented Dec 26, 2016

Prerequisites

  • I have read the documentation;
  • In the case of a bug report, I understand that providing a SSCCE example is tremendously useful to the maintainers.

Steps to Reproduce

  1. Navigate to "Numbers" tab in https://mozilla-services.github.io/react-jsonschema-form/
  2. Change 3.14 to 3.10
  3. Change to 3.0 or any other decimal ending in zero
  4. Note that it is not possible

Expected behavior

Don't throw type error for 3.10

Actual behavior

Type error thrown for 3.10

@n1k0 n1k0 added the bug label Dec 29, 2016
@Unsfer
Copy link

Unsfer commented Feb 7, 2017

+1

@glasserc
Copy link
Contributor

glasserc commented Feb 9, 2017

This is due to https://github.com/mozilla-services/react-jsonschema-form/blob/master/src/utils.js#L210-L215. It seems to me that there are two use cases here -- one for numeric values for which a trailing zero is semantically meaningless (and so we should allow it), and one for fixed-point numbers for which a trailing zero is meaningful, like for dollars-and-cents amounts. NumberField corresponds to the JSON type "number", which means that it matches the first use case. I'd like to reserve the ability to use fields for fixed-width decimal formats, but I don't see how we could deserialize its data as anything except string, and if you use a StringField, then I think that already covers this case. Maybe you'd want to add a custom validator to prevent things like 3.0.1 or 3.012. So I'd be OK with simplifying asNumber and adding a "decimal field" example to the documentation/playground.

@n1k0
Copy link
Collaborator

n1k0 commented Feb 13, 2017

The problem is trailing zeros are not preserved when serializing to JSON:

> JSON.parse("3.10")
3.1
> JSON.parse(JSON.stringify(3.10))
3.1
> 3.10
3.1

This library relies on JSON schema, hence on the JSON spec wrt value validation. My take is if you want trailing zeros in your numbers, you may actually want strings instead. I know this is not ideal, but I don't have any other idea how to preserve trailing zeros in numbers when the language & spec themselves don't allow it.

@n1k0 n1k0 closed this as completed Feb 13, 2017
@Natim
Copy link
Contributor

Natim commented Feb 13, 2017

Also another solution if you always want two digits after the comma would be to make to multiply the value by 100 (round it to its integer value) and then devide it by 100 and it will put it back.

@maxieduncan
Copy link

I've hit this issue recently as well. I don't need the trailing 0 to be preserved but it would be good if what is clearly a numeric value doesn't fail validation with the error "should be number"

@travisdahl
Copy link
Contributor

travisdahl commented Apr 18, 2019

Has anyone had any updates on this, in our use case we need to maintain the trailing 0s. But even if that wasnt supported I would expect it to strip them and move on.

But like mentioned above

... it would be good if what is clearly a numeric value doesn't fail validation with the error "should be number"

that seems like a bug.

@epicfaace
Copy link
Member

@travisdahl I believe this was solved in #1183 and is included in the latest release.

@travisdahl
Copy link
Contributor

@epicfaace my bad, we upgraded and saw the fix. It still drops the zeros but besides that it seems to be working

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

No branches or pull requests

8 participants