From a65d82130cf9c1902d86fa93ee6a15f2d53315a7 Mon Sep 17 00:00:00 2001 From: digiwand <20778143+digiwand@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:24:35 +0300 Subject: [PATCH 1/5] feat: display ellipsis if > 15 digits permit amounts --- .../info/row/text-token-units.test.tsx | 31 ++++++ .../app/confirm/info/row/text-token-units.tsx | 3 +- .../permit-simulation/permit-simulation.tsx | 3 +- .../simulation-details/formatAmount.test.ts | 103 +++++++++++------- .../simulation-details/formatAmount.ts | 40 +++++++ 5 files changed, 136 insertions(+), 44 deletions(-) diff --git a/ui/components/app/confirm/info/row/text-token-units.test.tsx b/ui/components/app/confirm/info/row/text-token-units.test.tsx index ee352d57acce..025c5ae8179d 100644 --- a/ui/components/app/confirm/info/row/text-token-units.test.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.test.tsx @@ -14,6 +14,17 @@ describe('ConfirmInfoRowTextTokenUnits', () => { expect(getByText('0.0123')).toBeInTheDocument(); }); + it('renders the value with the correct formatted number for lengthy decimals', () => { + const value = '300012312312123121'; + const decimals = 18; + const { getByText } = render( + , + ); + + // Note: using formatAmount loses precision + expect(getByText('0.3')).toBeInTheDocument(); + }); + it('renders the value with the correct formatted non-fractional number', () => { const value = 123456789; const decimals = 4; @@ -24,4 +35,24 @@ describe('ConfirmInfoRowTextTokenUnits', () => { // Note: using formatAmount loses precision expect(getByText('12,346')).toBeInTheDocument(); }); + + it('renders the value with the correct formatted number', () => { + const value = '30001231231212312138768'; + const decimals = 9; + const { getByText } = render( + , + ); + + expect(getByText('30,001,231,231,212')).toBeInTheDocument(); + }); + + it('renders the value with the correct formatted number and ellipsis', () => { + const value = '30001231231212312138768'; + const decimals = 7; + const { getByText } = render( + , + ); + + expect(getByText('3,000,123,123,121,23...')).toBeInTheDocument(); + }); }); diff --git a/ui/components/app/confirm/info/row/text-token-units.tsx b/ui/components/app/confirm/info/row/text-token-units.tsx index 62d0bb38e664..3db533e4d832 100644 --- a/ui/components/app/confirm/info/row/text-token-units.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.tsx @@ -3,6 +3,7 @@ import { BigNumber } from 'bignumber.js'; import { calcTokenAmount } from '../../../../../../shared/lib/transactions-controller-utils'; import { + ellipsisAmountText, formatAmount, formatAmountMaxPrecision, } from '../../../../../pages/confirmations/components/simulation-details/formatAmount'; @@ -26,7 +27,7 @@ export const ConfirmInfoRowTextTokenUnits: React.FC< return ( ); diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx index 15afe1e455f0..0510758c9b2f 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx @@ -25,6 +25,7 @@ import { SignatureRequestType } from '../../../../../types/confirm'; import useTokenExchangeRate from '../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate'; import { IndividualFiatDisplay } from '../../../../simulation-details/fiat-display'; import { + ellipsisAmountText, formatAmount, formatAmountMaxPrecision, } from '../../../../simulation-details/formatAmount'; @@ -95,7 +96,7 @@ const PermitSimulation: React.FC<{ textAlign={TextAlign.Center} ellipsis > - {tokenValue} + {ellipsisAmountText(tokenValue || '')} diff --git a/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts b/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts index 7f73e0ab42fc..4a39cc5dfa15 100644 --- a/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts +++ b/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts @@ -1,51 +1,70 @@ import { BigNumber } from 'bignumber.js'; -import { formatAmount } from './formatAmount'; +import { ellipsisAmountText, formatAmount } from './formatAmount'; describe('formatAmount', () => { - const locale = 'en-US'; + describe('#ellipsisAmountText', () => { + const MOCK_MAX_LEFT_DIGITS = 15; - it('returns "0" for zero amount', () => { - expect(formatAmount(locale, new BigNumber(0))).toBe('0'); + // @ts-expect-error This is missing from the Mocha type definitions + it.each([ + ['1.003', '1.003'], + ['1,034', '1,034'], + ['1,213,098,292,340,945', '1,213,098,292,340,94...'], + ['30,001,231,231,212,312,138,768', '30,001,231,231,212,3...'], + ])( + 'formats amount greater than or equal to 1 with appropriate decimal precision (%s => %s)', + (amount: string, expected: string) => { + expect(ellipsisAmountText(amount, MOCK_MAX_LEFT_DIGITS)).toBe(expected); + }, + ); }); - it('returns "<0.000001" for 0 < amount < MIN_AMOUNT', () => { - expect(formatAmount(locale, new BigNumber(0.0000009))).toBe('<0.000001'); - }); + describe('#formatAmount', () => { + const locale = 'en-US'; + + it('returns "0" for zero amount', () => { + expect(formatAmount(locale, new BigNumber(0))).toBe('0'); + }); - // @ts-expect-error This is missing from the Mocha type definitions - it.each([ - [0.0000456, '0.000046'], - [0.0004567, '0.000457'], - [0.003456, '0.00346'], - [0.023456, '0.0235'], - [0.125456, '0.125'], - ])( - 'formats amount less than 1 with maximum significant digits (%s => %s)', - (amount: number, expected: string) => { - expect(formatAmount(locale, new BigNumber(amount))).toBe(expected); - }, - ); + it('returns "<0.000001" for 0 < amount < MIN_AMOUNT', () => { + expect(formatAmount(locale, new BigNumber(0.0000009))).toBe('<0.000001'); + }); - // @ts-expect-error This is missing from the Mocha type definitions - it.each([ - [1.0034, '1.003'], - [1.034, '1.034'], - [1.3034, '1.303'], - [12.0345, '12.03'], - [121.456, '121.5'], - [1034.123, '1,034'], - [47361034.006, '47,361,034'], - ['12130982923409.5', '12,130,982,923,410'], - ['1213098292340944.5', '1,213,098,292,340,945'], - ['30001231231212312138768', '30,001,231,231,212,312,138,768'], - [ - '1157920892373161954235709850086879078532699846656405640394575.84007913129639935', - '1,157,920,892,373,161,954,235,709,850,086,879,078,532,699,846,656,405,640,394,576', - ], - ])( - 'formats amount greater than or equal to 1 with appropriate decimal precision (%s => %s)', - (amount: number, expected: string) => { - expect(formatAmount(locale, new BigNumber(amount))).toBe(expected); - }, - ); + // @ts-expect-error This is missing from the Mocha type definitions + it.each([ + [0.0000456, '0.000046'], + [0.0004567, '0.000457'], + [0.003456, '0.00346'], + [0.023456, '0.0235'], + [0.125456, '0.125'], + ])( + 'formats amount less than 1 with maximum significant digits (%s => %s)', + (amount: number, expected: string) => { + expect(formatAmount(locale, new BigNumber(amount))).toBe(expected); + }, + ); + + // @ts-expect-error This is missing from the Mocha type definitions + it.each([ + [1.0034, '1.003'], + [1.034, '1.034'], + [1.3034, '1.303'], + [12.0345, '12.03'], + [121.456, '121.5'], + [1034.123, '1,034'], + [47361034.006, '47,361,034'], + ['12130982923409.5', '12,130,982,923,410'], + ['1213098292340944.5', '1,213,098,292,340,945'], + ['30001231231212312138768', '30,001,231,231,212,312,138,768'], + [ + '1157920892373161954235709850086879078532699846656405640394575.84007913129639935', + '1,157,920,892,373,161,954,235,709,850,086,879,078,532,699,846,656,405,640,394,576', + ], + ])( + 'formats amount greater than or equal to 1 with appropriate decimal precision (%s => %s)', + (amount: number, expected: string) => { + expect(formatAmount(locale, new BigNumber(amount))).toBe(expected); + }, + ); + }); }); diff --git a/ui/pages/confirmations/components/simulation-details/formatAmount.ts b/ui/pages/confirmations/components/simulation-details/formatAmount.ts index 213b55d52083..8e63c44aa0bc 100644 --- a/ui/pages/confirmations/components/simulation-details/formatAmount.ts +++ b/ui/pages/confirmations/components/simulation-details/formatAmount.ts @@ -4,11 +4,51 @@ import { DEFAULT_PRECISION, } from '../../../../hooks/useCurrencyDisplay'; +const MAX_ELLIPSIS_LEFT_DIGITS = 15; + // The number of significant decimals places to show for amounts less than 1. const MAX_SIGNIFICANT_DECIMAL_PLACES = 3; const ZERO_DISPLAY = '0'; +/** + * This function receives an formatted number and will append an ellipsis if the number of digits + * is greater than MAX_LEFT_DIGITS. Currently, we're only supporting en-US format. When we support + * i18n numbers, we'll need to update this method to support i18n. + * + * This method has been designed to receive results of formatAmount. + * + * There is no need to format the decimal portion because formatAmount shaves the portions off + * accordingly. + * + * @param amountText + * @param maxLeftDigits + */ +export function ellipsisAmountText( + amountText: string, + maxLeftDigits: number = MAX_ELLIPSIS_LEFT_DIGITS, +): string { + const [integerPart] = amountText.split('.'); + const cleanIntegerPart = integerPart.replace(/,/gu, ''); + + if (cleanIntegerPart.length > maxLeftDigits) { + let result = ''; + let digitCount = 0; + + for (let i = 0; digitCount < maxLeftDigits; i++) { + const integerChar = integerPart[i]; + result += integerChar; + if (integerChar !== ',') { + digitCount += 1; + } + } + + return `${result}...`; + } + + return amountText; +} + export function formatAmountMaxPrecision( locale: string, num: number | BigNumber, From e2a9718c2a0b2196dbd30142253f7209613d3203 Mon Sep 17 00:00:00 2001 From: digiwand <20778143+digiwand@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:25:34 +0300 Subject: [PATCH 2/5] fix: rm no longer needed css ellipsis --- ui/components/app/confirm/info/row/text-token-units.tsx | 1 - .../info/typed-sign/permit-simulation/permit-simulation.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/ui/components/app/confirm/info/row/text-token-units.tsx b/ui/components/app/confirm/info/row/text-token-units.tsx index 3db533e4d832..610016d8f5d3 100644 --- a/ui/components/app/confirm/info/row/text-token-units.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.tsx @@ -26,7 +26,6 @@ export const ConfirmInfoRowTextTokenUnits: React.FC< return ( diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx index 0510758c9b2f..dbd5d0d2dc02 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx @@ -94,7 +94,6 @@ const PermitSimulation: React.FC<{ borderRadius={BorderRadius.XL} paddingInline={2} textAlign={TextAlign.Center} - ellipsis > {ellipsisAmountText(tokenValue || '')} From 706d6d9183b64f4d1cfd7f29fd7d52b607298713 Mon Sep 17 00:00:00 2001 From: digiwand <20778143+digiwand@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:27:49 +0300 Subject: [PATCH 3/5] refactor: remove unused isEllipsis from ConfirmInfoRowText --- ui/components/app/confirm/info/row/text.tsx | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/ui/components/app/confirm/info/row/text.tsx b/ui/components/app/confirm/info/row/text.tsx index cd30c3762e72..5b74776225fd 100644 --- a/ui/components/app/confirm/info/row/text.tsx +++ b/ui/components/app/confirm/info/row/text.tsx @@ -17,18 +17,8 @@ import { } from '../../../../component-library'; import Tooltip from '../../../../ui/tooltip'; -const InfoText = ({ - isEllipsis, - text, -}: { - isEllipsis: boolean; - text: string; -}) => ( - +const InfoText = ({ text }: { text: string }) => ( + {text} ); @@ -37,14 +27,12 @@ export type ConfirmInfoRowTextProps = { text: string; onEditClick?: () => void; editIconClassName?: string; - isEllipsis?: boolean; tooltip?: string; }; export const ConfirmInfoRowText: React.FC = ({ text, onEditClick, - isEllipsis = false, editIconClassName, tooltip, }) => { @@ -67,10 +55,10 @@ export const ConfirmInfoRowText: React.FC = ({ wrapperStyle={{ minWidth: 0 }} interactive > - + ) : ( - + )} {isEditable ? ( Date: Tue, 30 Jul 2024 19:06:36 +0300 Subject: [PATCH 4/5] test:fix: snapshots related to permit ellipsis --- .../info/typed-sign/__snapshots__/typed-sign.test.tsx.snap | 3 ++- .../__snapshots__/permit-simulation.test.tsx.snap | 2 +- .../confirmations/confirm/__snapshots__/confirm.test.tsx.snap | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap index 67596d6dac2b..3cf251d10584 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap @@ -409,7 +409,8 @@ exports[`TypedSignInfo correctly renders permit sign type 1`] = ` tabindex="0" >

3,000

diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap index 10e985076186..1de065afdbaa 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap @@ -80,7 +80,7 @@ exports[`PermitSimulation renders component correctly 1`] = ` tabindex="0" >

30 diff --git a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap index 64eed06931de..9255ae9ba0d4 100644 --- a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap +++ b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap @@ -1675,7 +1675,8 @@ exports[`Confirm should match snapshot for signature - typed sign - permit 1`] = tabindex="0" >

3,000

From 566166efc55216084ee521739c5de3d980e5bff1 Mon Sep 17 00:00:00 2001 From: digiwand <20778143+digiwand@users.noreply.github.com> Date: Tue, 30 Jul 2024 19:46:10 +0300 Subject: [PATCH 5/5] chore: rm stale comments --- ui/components/app/confirm/info/row/text-token-units.test.tsx | 3 --- ui/components/app/confirm/info/row/text-token-units.tsx | 2 -- 2 files changed, 5 deletions(-) diff --git a/ui/components/app/confirm/info/row/text-token-units.test.tsx b/ui/components/app/confirm/info/row/text-token-units.test.tsx index 025c5ae8179d..d25b23a52010 100644 --- a/ui/components/app/confirm/info/row/text-token-units.test.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.test.tsx @@ -10,7 +10,6 @@ describe('ConfirmInfoRowTextTokenUnits', () => { , ); - // Note: using formatAmount loses precision expect(getByText('0.0123')).toBeInTheDocument(); }); @@ -21,7 +20,6 @@ describe('ConfirmInfoRowTextTokenUnits', () => { , ); - // Note: using formatAmount loses precision expect(getByText('0.3')).toBeInTheDocument(); }); @@ -32,7 +30,6 @@ describe('ConfirmInfoRowTextTokenUnits', () => { , ); - // Note: using formatAmount loses precision expect(getByText('12,346')).toBeInTheDocument(); }); diff --git a/ui/components/app/confirm/info/row/text-token-units.tsx b/ui/components/app/confirm/info/row/text-token-units.tsx index 610016d8f5d3..bd7fbc52c195 100644 --- a/ui/components/app/confirm/info/row/text-token-units.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.tsx @@ -19,8 +19,6 @@ export const ConfirmInfoRowTextTokenUnits: React.FC< > = ({ value, decimals }) => { const tokenValue = calcTokenAmount(value, decimals); - // FIXME - Precision may be lost for large values when using formatAmount - /** @see {@link https://github.com/MetaMask/metamask-extension/issues/25755} */ const tokenText = formatAmount('en-US', tokenValue); const tokenTextMaxPrecision = formatAmountMaxPrecision('en-US', tokenValue);