From 74e2912bfc43151d1d759b53ed062f01a55cb050 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Thu, 25 Jul 2024 11:48:37 +0200 Subject: [PATCH] fix: add ellipsis for permit fiat values (#26001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR goal is to add ellipsis to the fiat value if it's more than 15 character in permit simulations. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26001?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2842 ## **Manual testing steps** 1. Go to cowswap 2. Swap a token with gas-free approve for another token 3. Notice the permit signature screen ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-07-22 at 14 40 57 ## **Pre-merge author checklist** - [X] 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). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] 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. --- ui/helpers/utils/util.js | 17 +++-- ui/helpers/utils/util.test.js | 11 +++ ui/hooks/useFiatFormatter.test.ts | 41 ++++++++++- ui/hooks/useFiatFormatter.ts | 73 +++++++++++++++++-- .../permit-simulation/permit-simulation.tsx | 4 +- .../simulation-details/amount-pill.tsx | 1 + .../simulation-details/fiat-display.tsx | 14 ++-- 7 files changed, 143 insertions(+), 18 deletions(-) diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index cae0ed769267..c68443d8c2fc 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -222,24 +222,30 @@ export function getRandomFileName() { * @param {number} options.truncatedCharLimit - The maximum length of the string. * @param {number} options.truncatedStartChars - The number of characters to preserve at the beginning. * @param {number} options.truncatedEndChars - The number of characters to preserve at the end. + * @param {boolean} options.skipCharacterInEnd - Skip the character at the end. * @returns {string} The shortened string. */ export function shortenString( stringToShorten = '', - { truncatedCharLimit, truncatedStartChars, truncatedEndChars } = { + { + truncatedCharLimit, + truncatedStartChars, + truncatedEndChars, + skipCharacterInEnd, + } = { truncatedCharLimit: TRUNCATED_NAME_CHAR_LIMIT, truncatedStartChars: TRUNCATED_ADDRESS_START_CHARS, truncatedEndChars: TRUNCATED_ADDRESS_END_CHARS, + skipCharacterInEnd: false, }, ) { if (stringToShorten.length < truncatedCharLimit) { return stringToShorten; } - return `${stringToShorten.slice( - 0, - truncatedStartChars, - )}...${stringToShorten.slice(-truncatedEndChars)}`; + return `${stringToShorten.slice(0, truncatedStartChars)}...${ + skipCharacterInEnd ? '' : stringToShorten.slice(-truncatedEndChars) + }`; } /** @@ -258,6 +264,7 @@ export function shortenAddress(address = '') { truncatedCharLimit: TRUNCATED_NAME_CHAR_LIMIT, truncatedStartChars: TRUNCATED_ADDRESS_START_CHARS, truncatedEndChars: TRUNCATED_ADDRESS_END_CHARS, + skipCharacterInEnd: false, }); } diff --git a/ui/helpers/utils/util.test.js b/ui/helpers/utils/util.test.js index 76811f432f81..2ad0db6e50a0 100644 --- a/ui/helpers/utils/util.test.js +++ b/ui/helpers/utils/util.test.js @@ -1081,5 +1081,16 @@ describe('util', () => { }), ).toStrictEqual('0x12...7890'); }); + + it('should shorten the string and remove all characters from the end if skipCharacterInEnd is true', () => { + expect( + util.shortenString('0x1234567890123456789012345678901234567890', { + truncatedCharLimit: 10, + truncatedStartChars: 4, + truncatedEndChars: 4, + skipCharacterInEnd: true, + }), + ).toStrictEqual('0x12...'); + }); }); }); diff --git a/ui/hooks/useFiatFormatter.test.ts b/ui/hooks/useFiatFormatter.test.ts index bf7fc670b5ed..7e6423f1df34 100644 --- a/ui/hooks/useFiatFormatter.test.ts +++ b/ui/hooks/useFiatFormatter.test.ts @@ -35,6 +35,44 @@ describe('useFiatFormatter', () => { expect(formatFiat(0)).toBe('$0.00'); }); + describe('shorten the fiat', () => { + it('when currency symbol on the left for given locale', () => { + mockGetIntlLocale.mockReturnValue('en-US'); + mockGetCurrentCurrency.mockReturnValue('USD'); + + const { result } = renderHook(() => useFiatFormatter()); + const formatFiat = result.current; + + expect(formatFiat(100000000000000000, { shorten: true })).toBe( + '$100,000,000,...', + ); + }); + + it('when currency symbol on the right for given locale', () => { + mockGetIntlLocale.mockReturnValue('es-ES'); + mockGetCurrentCurrency.mockReturnValue('EUR'); + + const { result } = renderHook(() => useFiatFormatter()); + const formatFiat = result.current; + + expect(formatFiat(100000000000000000, { shorten: true })).toBe( + '100.000.000....€', + ); + }); + + it('handle unknown currencies by returning amount followed by currency code', () => { + mockGetCurrentCurrency.mockReturnValue('storj'); + mockGetIntlLocale.mockReturnValue('en-US'); + + const { result } = renderHook(() => useFiatFormatter()); + const formatFiat = result.current; + + expect(formatFiat(100000, { shorten: true })).toBe('100,000 storj'); + expect(formatFiat(500.5, { shorten: true })).toBe('500.5 storj'); + expect(formatFiat(0, { shorten: true })).toBe('0 storj'); + }); + }); + it('should use the current locale and currency from the mocked functions', () => { mockGetIntlLocale.mockReturnValue('fr-FR'); mockGetCurrentCurrency.mockReturnValue('EUR'); @@ -47,12 +85,13 @@ describe('useFiatFormatter', () => { it('should gracefully handle unknown currencies by returning amount followed by currency code', () => { mockGetCurrentCurrency.mockReturnValue('storj'); + mockGetIntlLocale.mockReturnValue('en-US'); const { result } = renderHook(() => useFiatFormatter()); const formatFiat = result.current; // Testing the fallback formatting for an unknown currency - expect(formatFiat(1000)).toBe('1000 storj'); + expect(formatFiat(100000)).toBe('100,000 storj'); expect(formatFiat(500.5)).toBe('500.5 storj'); expect(formatFiat(0)).toBe('0 storj'); }); diff --git a/ui/hooks/useFiatFormatter.ts b/ui/hooks/useFiatFormatter.ts index 0f5e37151670..0c9bd33ffee5 100644 --- a/ui/hooks/useFiatFormatter.ts +++ b/ui/hooks/useFiatFormatter.ts @@ -1,35 +1,98 @@ import { useSelector } from 'react-redux'; import { getIntlLocale } from '../ducks/locale/locale'; import { getCurrentCurrency } from '../selectors'; +import { shortenString } from '../helpers/utils/util'; /** * Returns a function that formats a fiat amount as a localized string. + * The hook takes an optional options object to configure the formatting. * * Example usage: * * ``` * const formatFiat = useFiatFormatter(); * const formattedAmount = formatFiat(1000); + * + * const shorteningFiatFormatter = useFiatFormatter(); + * const shortenedAmount = shorteningFiatFormatter(100000000000000000, { shorten: true }); * ``` * * @returns A function that takes a fiat amount as a number and returns a formatted string. */ -type FiatFormatter = (fiatAmount: number) => string; +const TRUNCATED_CHAR_LIMIT_FOR_SHORTENED_FIAT = 15; +const TRUNCATED_START_CHARS_FOR_SHORTENED_FIAT = 12; + +type FiatFormatterOptions = { + shorten?: boolean; +}; + +type FiatFormatter = ( + fiatAmount: number, + options?: FiatFormatterOptions, +) => string; export const useFiatFormatter = (): FiatFormatter => { const locale = useSelector(getIntlLocale); const fiatCurrency = useSelector(getCurrentCurrency); - return (fiatAmount: number) => { + return (fiatAmount: number, options: FiatFormatterOptions = {}) => { + const { shorten } = options; + try { - return new Intl.NumberFormat(locale, { + const formatter = new Intl.NumberFormat(locale, { style: 'currency', currency: fiatCurrency, - }).format(fiatAmount); + }); + + if (!shorten) { + return formatter.format(fiatAmount); + } + + const parts = formatter.formatToParts(fiatAmount); + + let currencySymbol = ''; + let numberString = ''; + + parts.forEach((part) => { + if (part.type === 'currency') { + currencySymbol += part.value; + } else { + numberString += part.value; + } + }); + + // Shorten the number part while preserving commas + const shortenedNumberString = shortenString(numberString, { + truncatedCharLimit: TRUNCATED_CHAR_LIMIT_FOR_SHORTENED_FIAT, + truncatedStartChars: TRUNCATED_START_CHARS_FOR_SHORTENED_FIAT, + truncatedEndChars: 0, + skipCharacterInEnd: true, + }); + + // Determine the position of the currency symbol + const currencyBeforeNumber = + parts.findIndex((part) => part.type === 'currency') < + parts.findIndex((part) => part.type === 'integer'); + + // Reassemble the formatted string + return currencyBeforeNumber + ? `${currencySymbol}${shortenedNumberString}` + : `${shortenedNumberString}${currencySymbol}`; } catch (error) { // Fallback for unknown or unsupported currencies - return `${fiatAmount} ${fiatCurrency}`; + const formattedNumber = new Intl.NumberFormat(locale).format(fiatAmount); + const shortenedNumberString = shortenString(formattedNumber, { + truncatedCharLimit: TRUNCATED_CHAR_LIMIT_FOR_SHORTENED_FIAT, + truncatedStartChars: TRUNCATED_START_CHARS_FOR_SHORTENED_FIAT, + truncatedEndChars: 0, + skipCharacterInEnd: true, + }); + + if (shorten) { + return `${shortenedNumberString} ${fiatCurrency}`; + } + return `${formattedNumber} ${fiatCurrency}`; } }; }; 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 dbbe560ce0d9..b2a036370e10 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 @@ -97,7 +97,9 @@ const PermitSimulation: React.FC<{ - {fiatValue && } + {fiatValue && ( + + )} diff --git a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx index 5e86018f5e58..a8ad71cfadc6 100644 --- a/ui/pages/confirmations/components/simulation-details/amount-pill.tsx +++ b/ui/pages/confirmations/components/simulation-details/amount-pill.tsx @@ -59,6 +59,7 @@ export const AmountPill: React.FC<{ truncatedCharLimit: 11, truncatedStartChars: 4, truncatedEndChars: 4, + skipCharacterInEnd: false, }); const shortenedTokenIdPart = `#${shortenedDecimalTokenId}`; diff --git a/ui/pages/confirmations/components/simulation-details/fiat-display.tsx b/ui/pages/confirmations/components/simulation-details/fiat-display.tsx index 5143cd5ed06b..66e9148c676e 100644 --- a/ui/pages/confirmations/components/simulation-details/fiat-display.tsx +++ b/ui/pages/confirmations/components/simulation-details/fiat-display.tsx @@ -32,12 +32,14 @@ export function calculateTotalFiat(fiatAmounts: FiatAmount[]): number { /** * Displays the fiat value of a single balance change. * - * @param props - * @param props.fiatAmount + * @param props - The props object. + * @param props.fiatAmount - The fiat amount to display. + * @param props.shorten - Whether to shorten the fiat amount. */ -export const IndividualFiatDisplay: React.FC<{ fiatAmount: FiatAmount }> = ({ - fiatAmount, -}) => { +export const IndividualFiatDisplay: React.FC<{ + fiatAmount: FiatAmount; + shorten?: boolean; +}> = ({ fiatAmount, shorten = false }) => { const hideFiatForTestnet = useHideFiatForTestnet(); const fiatFormatter = useFiatFormatter(); @@ -50,7 +52,7 @@ export const IndividualFiatDisplay: React.FC<{ fiatAmount: FiatAmount }> = ({ } const absFiat = Math.abs(fiatAmount); - return {fiatFormatter(absFiat)}; + return {fiatFormatter(absFiat, { shorten })}; }; /**