-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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: Reduce usage of scientific notation #27992
base: develop
Are you sure you want to change the base?
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. |
6464543
to
2394961
Compare
Builds ready [2394961]
Page Load Metrics (1866 ± 60 ms)
Bundle size diffs
|
2394961
to
8dd6ae4
Compare
Builds ready [8dd6ae4]
Page Load Metrics (1887 ± 100 ms)
|
fiatDisplayValue, | ||
pending, | ||
}; | ||
}; | ||
|
||
export function roundDisplayValue(decodedTransferValue: number): string { |
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.
Is this is the same logic as ui/pages/confirmations/components/simulation-details/formatAmount.ts
?
If so, we definitely want to centralise this and as much as possible in regard to formatting and rendering large values as it's so often a cause of issues.
781e0d3
to
f42b721
Compare
f42b721
to
c805139
Compare
|
||
// keep in scientific notation | ||
return num.toString(); | ||
} |
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.
Will be nice to extract this to a util.
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 extracted the logic into a contained function but since this function is only used on this hook I think it makes sense for it to live on the same file for now.
Builds ready [c805139]
Page Load Metrics (2123 ± 112 ms)
Bundle size diffs
|
Description
Implements the decimals rounding strategy outlined in this document.
When the transfer value is rounded, a tooltip is shown with the full value without rounding. If the number has up to 18 decimal places, it is shown in a user readable format, without rounding. If the number has more than 18 decimal places, the tooltip defaults to showing the scientific notation format. This scenario is unlikely, and only possible through dApp API initiated transfers.
Includes unit tests.
Fixes: #27971
Related issues
Fixes: #27971
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist