Skip to content

Commit

Permalink
fix: add ellipsis for permit fiat values (#26001)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
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: MetaMask/MetaMask-planning#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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<img width="472" alt="Screenshot 2024-07-22 at 14 40 57"
src="https://github.com/user-attachments/assets/c5879989-7719-4e4b-902f-1f57144d8889">


## **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.
  • Loading branch information
OGPoyraz authored and digiwand committed Aug 19, 2024
1 parent 621d39c commit 74e2912
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 18 deletions.
17 changes: 12 additions & 5 deletions ui/helpers/utils/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}`;
}

/**
Expand All @@ -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,
});
}

Expand Down
11 changes: 11 additions & 0 deletions ui/helpers/utils/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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...');
});
});
});
41 changes: 40 additions & 1 deletion ui/hooks/useFiatFormatter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
});
Expand Down
73 changes: 68 additions & 5 deletions ui/hooks/useFiatFormatter.ts
Original file line number Diff line number Diff line change
@@ -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}`;
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ const PermitSimulation: React.FC<{
<Name value={verifyingContract} type={NameType.ETHEREUM_ADDRESS} />
</Box>
<Box>
{fiatValue && <IndividualFiatDisplay fiatAmount={fiatValue} />}
{fiatValue && (
<IndividualFiatDisplay fiatAmount={fiatValue} shorten />
)}
</Box>
</Box>
</ConfirmInfoRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const AmountPill: React.FC<{
truncatedCharLimit: 11,
truncatedStartChars: 4,
truncatedEndChars: 4,
skipCharacterInEnd: false,
});

const shortenedTokenIdPart = `#${shortenedDecimalTokenId}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -50,7 +52,7 @@ export const IndividualFiatDisplay: React.FC<{ fiatAmount: FiatAmount }> = ({
}
const absFiat = Math.abs(fiatAmount);

return <Text {...textStyle}>{fiatFormatter(absFiat)}</Text>;
return <Text {...textStyle}>{fiatFormatter(absFiat, { shorten })}</Text>;
};

/**
Expand Down

0 comments on commit 74e2912

Please sign in to comment.