[NumericInput] Support arbitrary precision inferred from smallest step size #833
Conversation
| } | ||
|
|
||
| private toMaxPrecision(value: number) { | ||
| let { stepMaxPrecision } = this.state; |
| const { stepMaxPrecision } = this.state; | ||
| // round the value to have the specified maximum precision (toFixed is the wrong choice, | ||
| // because it would show trailing zeros in the decimal part out to the specified precision) | ||
| return +(Math.round(parseFloat(value + "e+" + stepMaxPrecision)) + "e-" + stepMaxPrecision); |
There was a problem hiding this comment.
using parseFloat and + in the same sentence, nice. how about parseFloat twice though, for clarity?
There was a problem hiding this comment.
Ah, yep, makes sense.
| const { stepMaxPrecision } = this.state; | ||
| // round the value to have the specified maximum precision (toFixed is the wrong choice, | ||
| // because it would show trailing zeros in the decimal part out to the specified precision) | ||
| // source: http://stackoverflow.com/a/18358056/5199574 |
| stepMaxPrecision = Math.max(stepMaxPrecision, Utils.countDecimalPlaces(props.minorStepSize)); | ||
| } | ||
|
|
||
| return stepMaxPrecision; |
There was a problem hiding this comment.
what if you did like:
if (minorStepSize) { return countDecimalPlaces(minorStepSize); }
else { return countDecimalPlaces(stepSize); }then we only count maximum once. and the minorStepSize should be the smallest one, right? so we can just go off that.
There was a problem hiding this comment.
Sweet, I like that better. 👍 This was a relic from a few commits ago, when I considered adding a configurable precision prop that we'd have to sanitize.
| // round the value to have the specified maximum precision (toFixed is the wrong choice, | ||
| // because it would show trailing zeros in the decimal part out to the specified precision) | ||
| // source: http://stackoverflow.com/a/18358056/5199574 | ||
| return parseFloat(Math.round(parseFloat(value + "e+" + stepMaxPrecision)) + "e-" + stepMaxPrecision); |
There was a problem hiding this comment.
this math is frightening and possibly expensive, but i get it. also the experience is sweet as the precision adjusts automatically.
There was a problem hiding this comment.
Voila: https://jsperf.com/roundtomaxprecision. Will tweak this line based on these findings.
| } | ||
|
|
||
| private toMaxPrecision(value: number) { | ||
| const { stepMaxPrecision } = this.state; | ||
| // round the value to have the specified maximum precision (toFixed is the wrong choice, | ||
| // because it would show trailing zeros in the decimal part out to the specified precision) | ||
| // source: http://stackoverflow.com/a/18358056/5199574 |
There was a problem hiding this comment.
update this source link? points to old code
There was a problem hiding this comment.
I left it because it still captures the original idea, even though I made syntactic changes. Should I just remove it?
Fixes one sub-issue in #607
Checklist
Changes proposed in this pull request:
Currently, the most granular
NumericInputstep size of any kind (minor/major/regular) is 0.1. This PR adds support for arbitrarily small step sizes, adjusting the max precision of the displayed value accordingly (similar in intent to #832). As before, the precision of the displayed value shows the minimum number of decimal places necessary for each value (i.e.1displays as"1", not"1.000").Reviewers should focus on:
countDecimalPlacesfrom Slider labelPrecision #832, will reuse his exact implementation here once that PR merges.toMaxPrecisioncode feels a little wonky. There's probably a simpler way. Thoughts?