Skip to content

Commit 13a1fcf

Browse files
pedronfigueiredopedronfigueiredo
authored
feat: Display Unlimited for really large spending caps on Permit (#29102)
<!-- 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** Uses `UNLIMITED_THRESHOLD` to determine wether or not to show a permit amount as "Unlimited". Updates unit tests. <!-- 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/29102?quickstart=1) ## **Related issues** Fixes: MetaMask/MetaMask-planning#3763 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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/main/.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/main/.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.
1 parent 927ef8c commit 13a1fcf

File tree

5 files changed

+84
-33
lines changed

5 files changed

+84
-33
lines changed

‎ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions
88
import { getIntlLocale } from '../../../../../../../ducks/locale/locale';
99
import { formatAmount } from '../../../../simulation-details/formatAmount';
1010
import { useDecodedTransactionData } from '../../hooks/useDecodedTransactionData';
11+
import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants';
1112
import { useIsNFT } from './use-is-nft';
1213

13-
const UNLIMITED_THRESHOLD = 10 ** 15;
14-
1514
function isSpendingCapUnlimited(decodedSpendingCap: number) {
16-
return decodedSpendingCap >= UNLIMITED_THRESHOLD;
15+
return decodedSpendingCap >= TOKEN_VALUE_UNLIMITED_THRESHOLD;
1716
}
1817

1918
export const useApproveTokenSimulation = (

‎ui/pages/confirmations/components/confirm/info/shared/constants.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
export const HEX_ZERO = '0x0';
2+
3+
export const TOKEN_VALUE_UNLIMITED_THRESHOLD = 10 ** 15;

‎ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx

+30-1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,35 @@ const decodingDataBidding: DecodingDataStateChanges = [
7777

7878
describe('DecodedSimulation', () => {
7979
it('renders component correctly', async () => {
80+
const state = getMockTypedSignConfirmStateForRequest({
81+
...permitSignatureMsg,
82+
decodingLoading: false,
83+
decodingData: {
84+
...decodingData,
85+
stateChanges: decodingData.stateChanges
86+
? [
87+
{
88+
...decodingData.stateChanges[0],
89+
amount: '12345',
90+
},
91+
]
92+
: [],
93+
},
94+
});
95+
96+
const mockStore = configureMockStore([])(state);
97+
98+
const { findByText } = renderWithConfirmContextProvider(
99+
<PermitSimulation />,
100+
mockStore,
101+
);
102+
103+
expect(await findByText('Estimated changes')).toBeInTheDocument();
104+
expect(await findByText('Spending cap')).toBeInTheDocument();
105+
expect(await findByText('12,345')).toBeInTheDocument();
106+
});
107+
108+
it('renders component correctly for a very large amount', async () => {
80109
const state = getMockTypedSignConfirmStateForRequest({
81110
...permitSignatureMsg,
82111
decodingLoading: false,
@@ -91,7 +120,7 @@ describe('DecodedSimulation', () => {
91120

92121
expect(await findByText('Estimated changes')).toBeInTheDocument();
93122
expect(await findByText('Spending cap')).toBeInTheDocument();
94-
expect(await findByText('1,461,501,637,3...')).toBeInTheDocument();
123+
expect(await findByText('Unlimited')).toBeInTheDocument();
95124
});
96125

97126
it('render correctly for ERC712 token', async () => {

‎ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx

+5
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ const StateChangeRow = ({
7171
const { assetType, changeType, amount, contractAddress, tokenID } =
7272
stateChange;
7373
const tooltip = getStateChangeToolip(stateChangeList, stateChange, t);
74+
const canDisplayValueAsUnlimited =
75+
assetType === TokenStandard.ERC20 &&
76+
(changeType === DecodingDataChangeType.Approve ||
77+
changeType === DecodingDataChangeType.Revoke);
7478
return (
7579
<ConfirmInfoRow
7680
label={shouldDisplayLabel ? getStateChangeLabelMap(t, changeType) : ''}
@@ -86,6 +90,7 @@ const StateChangeRow = ({
8690
tokenId={tokenID}
8791
credit={changeType === DecodingDataChangeType.Receive}
8892
debit={changeType === DecodingDataChangeType.Transfer}
93+
canDisplayValueAsUnlimited={canDisplayValueAsUnlimited}
8994
/>
9095
)}
9196
{assetType === 'NATIVE' && (

‎ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx

+45-29
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,11 @@
1-
import React, { useMemo } from 'react';
21
import { NameType } from '@metamask/name-controller';
32
import { Hex } from '@metamask/utils';
43
import { captureException } from '@sentry/browser';
5-
4+
import React, { useMemo } from 'react';
65
import { MetaMetricsEventLocation } from '../../../../../../../../../shared/constants/metametrics';
7-
import { shortenString } from '../../../../../../../../helpers/utils/util';
86
import { calcTokenAmount } from '../../../../../../../../../shared/lib/transactions-controller-utils';
97
import useTokenExchangeRate from '../../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
10-
import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display';
11-
import {
12-
formatAmount,
13-
formatAmountMaxPrecision,
14-
} from '../../../../../simulation-details/formatAmount';
15-
import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails';
16-
import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation';
17-
8+
import Name from '../../../../../../../../components/app/name/name';
189
import {
1910
Box,
2011
Text,
@@ -27,8 +18,17 @@ import {
2718
JustifyContent,
2819
TextAlign,
2920
} from '../../../../../../../../helpers/constants/design-system';
30-
import Name from '../../../../../../../../components/app/name/name';
21+
import { shortenString } from '../../../../../../../../helpers/utils/util';
22+
import { useI18nContext } from '../../../../../../../../hooks/useI18nContext';
23+
import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails';
24+
import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation';
3125
import { TokenDetailsERC20 } from '../../../../../../utils/token';
26+
import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display';
27+
import {
28+
formatAmount,
29+
formatAmountMaxPrecision,
30+
} from '../../../../../simulation-details/formatAmount';
31+
import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../../shared/constants';
3232
import { getAmountColors } from '../../../utils';
3333

3434
type PermitSimulationValueDisplayParams = {
@@ -56,6 +56,9 @@ type PermitSimulationValueDisplayParams = {
5656

5757
/** True if value is being debited to wallet */
5858
debit?: boolean;
59+
60+
/** Whether a large amount can be substituted by "Unlimited" */
61+
canDisplayValueAsUnlimited?: boolean;
5962
};
6063

6164
const PermitSimulationValueDisplay: React.FC<
@@ -68,7 +71,10 @@ const PermitSimulationValueDisplay: React.FC<
6871
value,
6972
credit,
7073
debit,
74+
canDisplayValueAsUnlimited,
7175
}) => {
76+
const t = useI18nContext();
77+
7278
const exchangeRate = useTokenExchangeRate(tokenContract);
7379

7480
const tokenDetails = useGetTokenStandardAndDetails(tokenContract);
@@ -88,18 +94,26 @@ const PermitSimulationValueDisplay: React.FC<
8894
return undefined;
8995
}, [exchangeRate, tokenDecimals, value]);
9096

91-
const { tokenValue, tokenValueMaxPrecision } = useMemo(() => {
92-
if (!value || tokenId) {
93-
return { tokenValue: null, tokenValueMaxPrecision: null };
94-
}
97+
const { tokenValue, tokenValueMaxPrecision, shouldShowUnlimitedValue } =
98+
useMemo(() => {
99+
if (!value || tokenId) {
100+
return {
101+
tokenValue: null,
102+
tokenValueMaxPrecision: null,
103+
shouldShowUnlimitedValue: false,
104+
};
105+
}
95106

96-
const tokenAmount = calcTokenAmount(value, tokenDecimals);
107+
const tokenAmount = calcTokenAmount(value, tokenDecimals);
97108

98-
return {
99-
tokenValue: formatAmount('en-US', tokenAmount),
100-
tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount),
101-
};
102-
}, [tokenDecimals, value]);
109+
return {
110+
tokenValue: formatAmount('en-US', tokenAmount),
111+
tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount),
112+
shouldShowUnlimitedValue:
113+
canDisplayValueAsUnlimited &&
114+
Number(value) > TOKEN_VALUE_UNLIMITED_THRESHOLD,
115+
};
116+
}, [tokenDecimals, value]);
103117

104118
/** Temporary error capturing as we are building out Permit Simulations */
105119
if (!tokenContract) {
@@ -138,13 +152,15 @@ const PermitSimulationValueDisplay: React.FC<
138152
>
139153
{credit && '+ '}
140154
{debit && '- '}
141-
{tokenValue !== null &&
142-
shortenString(tokenValue || '', {
143-
truncatedCharLimit: 15,
144-
truncatedStartChars: 15,
145-
truncatedEndChars: 0,
146-
skipCharacterInEnd: true,
147-
})}
155+
{shouldShowUnlimitedValue
156+
? t('unlimited')
157+
: tokenValue !== null &&
158+
shortenString(tokenValue || '', {
159+
truncatedCharLimit: 15,
160+
truncatedStartChars: 15,
161+
truncatedEndChars: 0,
162+
skipCharacterInEnd: true,
163+
})}
148164
{tokenId && `#${tokenId}`}
149165
</Text>
150166
</Tooltip>

0 commit comments

Comments
 (0)