From 4ca39b4a18147add18be87a3b4b97cd9cf81f8c7 Mon Sep 17 00:00:00 2001 From: Ariella Vu <20778143+digiwand@users.noreply.github.com> Date: Fri, 16 Aug 2024 09:35:28 -0400 Subject: [PATCH] fix: Permit ellipsis should use max 15 char (#26458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Display max 15 characters, not max 15 digits, before ellipsis for amounts shown in Permit pages. Updated to use `shortenString` here. ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2845 ## **Manual testing steps** 1. Go to test-dapp 2. Trigger Malicious Permit request 3. Observe ellipsis values in a. spending cap in simulation b. value in the message ## **Screenshots/Recordings** ### **Before** ### **After** ![CleanShot 2024-08-15 at 21 08 45](https://github.com/user-attachments/assets/020957dd-9204-40b6-afa2-9afacb80d9e0) ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../info/row/text-token-units.test.tsx | 4 +- .../app/confirm/info/row/text-token-units.tsx | 9 ++++- .../permit-simulation/permit-simulation.tsx | 9 ++++- .../simulation-details/formatAmount.test.ts | 19 +-------- .../simulation-details/formatAmount.ts | 40 ------------------- 5 files changed, 17 insertions(+), 64 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 d25b23a52010..311ab0acd9c5 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 @@ -40,7 +40,7 @@ describe('ConfirmInfoRowTextTokenUnits', () => { , ); - expect(getByText('30,001,231,231,212')).toBeInTheDocument(); + expect(getByText('30,001,231,231,...')).toBeInTheDocument(); }); it('renders the value with the correct formatted number and ellipsis', () => { @@ -50,6 +50,6 @@ describe('ConfirmInfoRowTextTokenUnits', () => { , ); - expect(getByText('3,000,123,123,121,23...')).toBeInTheDocument(); + expect(getByText('3,000,123,123,1...')).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 bd7fbc52c195..cb0a620c4b5e 100644 --- a/ui/components/app/confirm/info/row/text-token-units.tsx +++ b/ui/components/app/confirm/info/row/text-token-units.tsx @@ -3,10 +3,10 @@ import { BigNumber } from 'bignumber.js'; import { calcTokenAmount } from '../../../../../../shared/lib/transactions-controller-utils'; import { - ellipsisAmountText, formatAmount, formatAmountMaxPrecision, } from '../../../../../pages/confirmations/components/simulation-details/formatAmount'; +import { shortenString } from '../../../../../helpers/utils/util'; import { ConfirmInfoRowText } from './text'; type ConfirmInfoRowTextTokenUnitsProps = { @@ -24,7 +24,12 @@ 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 dbd5d0d2dc02..16ad6e713728 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 @@ -10,6 +10,7 @@ import { ConfirmInfoRow, ConfirmInfoRowText, } from '../../../../../../../components/app/confirm/info/row'; +import { shortenString } from '../../../../../../../helpers/utils/util'; import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; import { currentConfirmationSelector } from '../../../../../../../selectors'; import { Box, Text } from '../../../../../../../components/component-library'; @@ -25,7 +26,6 @@ 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 +95,12 @@ const PermitSimulation: React.FC<{ paddingInline={2} textAlign={TextAlign.Center} > - {ellipsisAmountText(tokenValue || '')} + {shortenString(tokenValue || '', { + truncatedCharLimit: 15, + truncatedStartChars: 15, + truncatedEndChars: 0, + skipCharacterInEnd: true, + })} diff --git a/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts b/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts index 4a39cc5dfa15..4ae9b7ee7278 100644 --- a/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts +++ b/ui/pages/confirmations/components/simulation-details/formatAmount.test.ts @@ -1,24 +1,7 @@ import { BigNumber } from 'bignumber.js'; -import { ellipsisAmountText, formatAmount } from './formatAmount'; +import { formatAmount } from './formatAmount'; describe('formatAmount', () => { - describe('#ellipsisAmountText', () => { - const MOCK_MAX_LEFT_DIGITS = 15; - - // @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); - }, - ); - }); - describe('#formatAmount', () => { const locale = 'en-US'; diff --git a/ui/pages/confirmations/components/simulation-details/formatAmount.ts b/ui/pages/confirmations/components/simulation-details/formatAmount.ts index 8e63c44aa0bc..213b55d52083 100644 --- a/ui/pages/confirmations/components/simulation-details/formatAmount.ts +++ b/ui/pages/confirmations/components/simulation-details/formatAmount.ts @@ -4,51 +4,11 @@ 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,