feat: improvement for how we display big and small numbers#25438
feat: improvement for how we display big and small numbers#25438
Conversation
…sk-extension into permit_token_decimal_fix
92b0369 to
8c99c92
Compare
|
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. |
Builds ready [8c99c92]
Page Load Metrics (52 ± 4 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25438 +/- ##
========================================
Coverage 69.61% 69.61%
========================================
Files 1364 1364
Lines 48189 48199 +10
Branches 13297 13299 +2
========================================
+ Hits 33542 33549 +7
- Misses 14647 14650 +3 ☔ View full report in Codecov by Sentry. |
| export const DefaultStory = ({ text }) => <ConfirmInfoRowText text={text} />; | ||
| DefaultStory.args = { | ||
| text: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.', | ||
| tooltip: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.', |
There was a problem hiding this comment.
Minor, but is it worth a constant for this such as TEXT_MOCK?
| }, [exchangeRate, value]); | ||
|
|
||
| const { tokenValue, tokenValueMaxPrecision } = useMemo(() => { | ||
| const valueBN = new BigNumber(value / Math.pow(10, tokenDecimals)); |
There was a problem hiding this comment.
This is the same logic we do in the decimals PR, perhaps an applyDecimals util?
There was a problem hiding this comment.
A new util for just division does may not be very useful.
| ); | ||
| } | ||
| if (isPermit && label === 'value') { | ||
| const valueBN = new BigNumber( |
There was a problem hiding this comment.
Minor, but could we split this function up with some local components such as DataFieldNumber?
There was a problem hiding this comment.
There is more requirement for formatting I will do some refactoring in DataTree in upcoming PRs.
| import { | ||
| formatAmount, | ||
| formatAmountMaxPrecision, | ||
| } from '../../../../simulation-details/formatAmount'; |
There was a problem hiding this comment.
We could move this to a higher level util but I think this makes sense in this context since we're also a simulation component 👍
| const valueBN = new BigNumber(value / Math.pow(10, tokenDecimals)); | ||
| return { | ||
| tokenValue: formatAmount('en-US', valueBN), | ||
| tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', valueBN), |
There was a problem hiding this comment.
similar to the existing simulation UI, I think we should support varying locales rather than only en-US
There was a problem hiding this comment.
Note there were some issues with this code. We are addressing the issues here:
|
Builds ready [7507aab]
Page Load Metrics (200 ± 219 ms)
Bundle size diffs
|
|
related cherry-pick PR: #25720 |
Description
Improvement in how we display big and small token value on permit signature page.
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2649
Manual testing steps
Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist