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

Filter roundToDecimals fails parsing string values #1282

Closed
jacobwod opened this issue Feb 6, 2023 · 2 comments
Closed

Filter roundToDecimals fails parsing string values #1282

jacobwod opened this issue Feb 6, 2023 · 2 comments
Assignees
Labels
Milestone

Comments

@jacobwod
Copy link
Member

jacobwod commented Feb 6, 2023

Describe the bug
This: {GRAY_INDEX|roundToDecimals(2)} will fail if GRAY_INDEX is a string (which it is if it comes from a XML/GML response - only JSON responses have valid JS types).

The reason
I think that this

return parseFloat(value.toFixed(parseInt(numDecimals))).toLocaleString();

should be this:

return parseFloat(value).toFixed(parseInt(numDecimals)).toLocaleString();

I.e. first parse value, then invoke toFixed(). Currently, we invoke toFixed() on a value that might be a string. I don't think this was intended.

@jacobwod jacobwod added the bug label Feb 6, 2023
@jacobwod jacobwod added this to the 3.x milestone Feb 6, 2023
@jacobwod jacobwod self-assigned this Feb 6, 2023
@jacobwod jacobwod modified the milestones: 3.x, 3.12 Feb 6, 2023
@jacobwod jacobwod changed the title Filter 'roundToDecimals' fails parsing string values Filter roundToDecimals fails parsing string values Feb 6, 2023
@jesade-vbg
Copy link
Contributor

Actually I think it must be parsed to float again before toLocaleString, if you want toLocaleString to work.

let value = "8.2342"; // can be string or number
let numDecimals = "2"; // can be string or number

parseFloat(parseFloat(value).toFixed(parseInt(numDecimals))).toLocaleString();

@jacobwod
Copy link
Member Author

Or just use the Number construct? It has both toFixed and toLocaleString I'll leave it up to you. 😄

jesade-vbg added a commit that referenced this issue Feb 23, 2023
@jesade-vbg jesade-vbg self-assigned this Feb 23, 2023
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

2 participants