Number Fields use String value storage; allow step="any" attribute#30
Merged
hecrj merged 2 commits intohecrj:masterfrom Jul 11, 2019
Merged
Number Fields use String value storage; allow step="any" attribute#30hecrj merged 2 commits intohecrj:masterfrom
hecrj merged 2 commits intohecrj:masterfrom
Conversation
Step would probably be more elegantly modeled with a custom type — do we want to introduce that level of complexity?
hecrj
approved these changes
Jul 11, 2019
Owner
hecrj
left a comment
There was a problem hiding this comment.
This is great! Thank you!
I think a step of Nothing being translated to any makes sense. In the end, any means no step whatsoever.
Also, thank you for updating the changelog. I will release 8.0 soon!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: this PR requires bumping the package version, I believe it can go with the rest of the 8.0 changes currently in master. Should it miss 8.0, a new version bump would need to be added.
As discussed in #25, storing number fields' values as
Maybe Floats caused issues when entering decimal values. This updates number fields' value storage to useString.Also related to decimals in number fields, the HTML spec allows the step attribute to be set to "any" to allow arbitrary precision. (If the
stepattribute is not present on the number input, the default is a step value of 1 (Integers only)). Making use of the "any" value is of interest to our app.Accordingly, I changed the
stepitem in NumberField.Attributes fromnumbertoMaybe number-- The idea being specifyingNothingmeans any step value is allowed.Step could probably be more elegantly and clearly modeled with a custom type — do we want to introduce that level of complexity? Something like:
I feel like using Maybe is consistent with the rest of the Api design, but let me know if you prefer the custom type, I'm happy to add it if you'd prefer.