Skip to content

Commit

Permalink
feat: Update Redesign Signature Permit to show ellipsis at max 15 dig…
Browse files Browse the repository at this point in the history
…its (#26227)

<!--
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?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26227?quickstart=1)

## **Related issues**

Fixes: #26226
Fixes: MetaMask/MetaMask-planning#2730 (older,
original issue)

## **Manual testing steps**

1. Go to cowswap
2. Swap a token with gas-free approve for another token
3. Notice the permit signature screen

or

1. run `yarn storybook`
2. go to
http://localhost:6006/?path=/story/pages-confirmations-confirm-signatures-signedtypeddatav3orv4--permit-story&args=msgParams.data:3.0001231231212312e+22
3. copy and paste the following into data
```
"{\"types\":{\"EIP712Domain\":[{\"name\":\"name\",\"type\":\"string\"},{\"name\":\"version\",\"type\":\"string\"},{\"name\":\"chainId\",\"type\":\"uint256\"},{\"name\":\"verifyingContract\",\"type\":\"address\"}],\"Permit\":[{\"name\":\"owner\",\"type\":\"address\"},{\"name\":\"spender\",\"type\":\"address\"},{\"name\":\"value\",\"type\":\"uint256\"},{\"name\":\"nonce\",\"type\":\"uint256\"},{\"name\":\"deadline\",\"type\":\"uint256\"}]},\"primaryType\":\"Permit\",\"domain\":{\"name\":\"MyToken\",\"version\":\"1\",\"verifyingContract\":\"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC\",\"chainId\":1},\"message\":{\"owner\":\"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477\",\"spender\":\"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4\",\"value\":30001231231212312138768,\"nonce\":0,\"deadline\":50000000000}}"
```

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

<img width="520"
src="https://github.com/user-attachments/assets/1b6e45cb-f54b-45e5-968c-141359de7466">
<img width="320"
src="https://github.com/user-attachments/assets/804d4e8a-1c47-470e-88a4-c6195c5cf96e">

## **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.
  • Loading branch information
digiwand authored Aug 2, 2024
1 parent b22e018 commit f001d60
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 69 deletions.
32 changes: 30 additions & 2 deletions ui/components/app/confirm/info/row/text-token-units.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,46 @@ describe('ConfirmInfoRowTextTokenUnits', () => {
<ConfirmInfoRowTextTokenUnits value={value} decimals={decimals} />,
);

// Note: using formatAmount loses precision
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(
<ConfirmInfoRowTextTokenUnits value={value} decimals={decimals} />,
);

expect(getByText('0.3')).toBeInTheDocument();
});

it('renders the value with the correct formatted non-fractional number', () => {
const value = 123456789;
const decimals = 4;
const { getByText } = render(
<ConfirmInfoRowTextTokenUnits value={value} decimals={decimals} />,
);

// 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(
<ConfirmInfoRowTextTokenUnits value={value} decimals={decimals} />,
);

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(
<ConfirmInfoRowTextTokenUnits value={value} decimals={decimals} />,
);

expect(getByText('3,000,123,123,121,23...')).toBeInTheDocument();
});
});
6 changes: 2 additions & 4 deletions ui/components/app/confirm/info/row/text-token-units.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -18,15 +19,12 @@ 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);

return (
<ConfirmInfoRowText
isEllipsis={true}
text={tokenText}
text={ellipsisAmountText(tokenText)}
tooltip={tokenTextMaxPrecision}
/>
);
Expand Down
20 changes: 4 additions & 16 deletions ui/components/app/confirm/info/row/text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,8 @@ import {
} from '../../../../component-library';
import Tooltip from '../../../../ui/tooltip';

const InfoText = ({
isEllipsis,
text,
}: {
isEllipsis: boolean;
text: string;
}) => (
<Text
color={TextColor.inherit}
style={isEllipsis ? {} : { whiteSpace: 'pre-wrap' }}
ellipsis={isEllipsis}
>
const InfoText = ({ text }: { text: string }) => (
<Text color={TextColor.inherit} style={{ whiteSpace: 'pre-wrap' }}>
{text}
</Text>
);
Expand All @@ -37,14 +27,12 @@ export type ConfirmInfoRowTextProps = {
text: string;
onEditClick?: () => void;
editIconClassName?: string;
isEllipsis?: boolean;
tooltip?: string;
};

export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
text,
onEditClick,
isEllipsis = false,
editIconClassName,
tooltip,
}) => {
Expand All @@ -67,10 +55,10 @@ export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
wrapperStyle={{ minWidth: 0 }}
interactive
>
<InfoText isEllipsis={isEllipsis} text={text} />
<InfoText text={text} />
</Tooltip>
) : (
<InfoText isEllipsis={isEllipsis} text={text} />
<InfoText text={text} />
)}
{isEditable ? (
<ButtonIcon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ exports[`TypedSignInfo correctly renders permit sign type 1`] = `
tabindex="0"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--ellipsis mm-box--color-inherit"
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
>
3,000
</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ exports[`PermitSimulation renders component correctly 1`] = `
tabindex="0"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--ellipsis mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
data-testid="simulation-token-value"
>
30
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -93,9 +94,8 @@ const PermitSimulation: React.FC<{
borderRadius={BorderRadius.XL}
paddingInline={2}
textAlign={TextAlign.Center}
ellipsis
>
{tokenValue}
{ellipsisAmountText(tokenValue || '')}
</Text>
</Tooltip>
</Box>
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
},
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,8 @@ exports[`Confirm should match snapshot for signature - typed sign - permit 1`] =
tabindex="0"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--ellipsis mm-box--color-inherit"
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
>
3,000
</p>
Expand Down

0 comments on commit f001d60

Please sign in to comment.