-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add decimal formatting to fix floating point to string issues #247
base: master
Are you sure you want to change the base?
Conversation
…ring conversion issues (e.g. 94.399999999999999%)
I'm not sure why this is failing. Apparently the library you are using is packaged up as a 'module', which is something fairly new, and I'm guessing our tooling doesn't support it for one reason or another. I had hoped that this was an issue with travis not using a new enough version of node, but after upgrading node on my machine that doesn't seem to be the case. It could have to do with the fact that this site was built using a very old create-react-app version (1.x, they are at 3.x now), and the associated react-scripts are old (which drive our test funcitonality, etc). Upgrading react-scripts does not seem like a trivial task (there are manual steps to upgrade your codebase from 1.x to 2.x, then 2.x to 3.x). I started the process but ran into issues w/xcode dependencies on my mac (recently upgraded iOS version), and since I'm getting a new lifecycle in a couple of weeks I'm not going to spend more time on this now. Were you able to get the tests to pass on your own machine @mrnosal? Or is this a problem with the old libraries we are using as I fear? |
BTW I tried routing around this problem by doing It seems like our test environment (running in Node.js, through |
updating react-scripts to 1.1.5 causes .mjs to be resolved after .js files. Also fix graphviz to handle complex transitions where there is a named distribution. This is what was causing the injuries module to fail with new decimal formatting. |
The updated formatting does look nicer but I think this needs some extra error handling.
|
The issue of I can fix, but the tests for stringUtils are set to reject some strings that are valid to pass to parseFloat(). There are some slight differences between the number formats allowed by the JSON specification and the strings acceptable to parseFloat(). Since parseFloat() is more forgiving of the two, I suggest that I update stringUtils and its tests to be compatible with parseFloat. |
Fixes JS fp to string conversion issues (e.g. 94.399999999999999%)
Checks for significant digits in user entered values rather than using fixed number of decimal places.
Shows at least 1 decimal place for percentages (e.g. 3% becomes 3.0%. 0.5% means the remainder is shown as 99.5%. 0.05% gives a remainder of 99.95%, etc.)