-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix: fix percentage change for non standard currencies #27239
fix: fix percentage change for non standard currencies #27239
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
...ltichain/token-list-item/price/percentage-and-amount-change/percentage-and-amount-change.tsx
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
...options, | ||
minimumFractionDigits: 2, | ||
style: 'decimal', | ||
}).format(balanceChange as number)} `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With standard currency codes, Intl.NumberFormat
with currency
argument would return the right currency formatted exp "$10" or "€5";
With non standard currency codes, we could do something like
`.format(balanceChange as number)} ${fiatCurrency}``; which would result in displaying the name of the currency (screenshot)
I thought it is not necessary and we can display just the value for non standard currencies;
Can update if you guys think it looks better with currency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just the value is okay because the currency symbol is shown on the line above
Builds ready [4e884c5]
Page Load Metrics (1821 ± 83 ms)
Bundle size diffs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try/catch looks reasonable to me such that we aren't needing to account for what a browser would consider a valid code.
Description
PR that fixes error when users choose a currency that is non standard currency code.
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
Screen.Recording.2024-09-18.at.10.46.17.mov
After
Screen.Recording.2024-09-18.at.10.44.00.mov
Pre-merge author checklist
Pre-merge reviewer checklist