fix: handle undefined options in number abbrev#3860
fix: handle undefined options in number abbrev#3860gomesalexandre merged 5 commits intoshapeshift:developfrom
Conversation
|
This is a draft until Product gives their OK for altering the current values displayed by the Price Chart tooltip, I've asked them on Discord and will update when there is an answer. If the values must remain imprecise for some reason, I still think the underlying bug should be fixed, but then we'll need to set a "2" for the |
|
@firebomb1 you may get stomped on by the large @pastaghost here - he's got a very nice suggestion w.r.t how we handle asset values that may or may not capture this - have a chat to him |
|
@DiggyDiggy2 @reallybeard are ok with this conceptually, let's not block on @pastaghost |
Agreed. AssetService is going to take a day or two as I'm trying to button up the KeepKey work right now. |
|
@0xdef1cafe |
|
Thanks for the link, the new lib will deal with crypto value handling/formatting and the current issue/PR relate to fiat value formatting, so while complementary at some point (to get a fiat balance) they should not impact each other as far as I can see. |
gomesalexandre
left a comment
There was a problem hiding this comment.
Tested locally, LGTM! Great PR once again @firebomb1 🐐
Description
Handling of options which have an
unidentifiedvalue when abbreviating numbers (e.g. coming from components with optional props without default values) in order for them to not override the defaults set based on the number being formatted.This fixes a bug which was visible in the Price Chart tooltip, the number of fractional digits for low value assets was not dynamic and always set at the default value of
Intl.NumberFormatfor currencies (2) because the default was always overridden byundefined.The "10" default forThis was used because later the number is truncated instead of rounded, I didn't understand this at first.maximumFractionDigitsmentioned above is also removed to the benefit of the dynamic computation that was already done but not used either for a reason I don't know.Notice
Pull Request Type
Issue (if applicable)
closes #3854
Risk
Modifies the logic for fiat amounts formatting across the whole app.
Testing
Engineering
Operations
Screenshots (if applicable)
Price Chart tooltip after the fix:
2023-02-17_11-41-18.webm