Skip to content

Commit

Permalink
feat: "Unlimited" value Decoding Simulation and account and message m…
Browse files Browse the repository at this point in the history
…odal UI/UX updates (#13030)

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

Feat:
- Support "Unlimited" display values in decoding simulation
- Update account detail and message modals to have transparent
backgrounds and enable click to close on background


Note: It might be helpful to walk through the commits to review

## **Related issues**

Fixes: #13022
Relates to: #12994

## **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**

![CleanShot 2025-01-16 at 03 23
30](https://github.com/user-attachments/assets/fd192b87-c5b7-4bb8-9d35-509b8c6bd55a)
![CleanShot 2025-01-16 at 04 12
30](https://github.com/user-attachments/assets/3468d59b-f045-4f5a-8669-09de78b7b7d1)

### **After**

![CleanShot 2025-01-16 at 03 22
54](https://github.com/user-attachments/assets/4a02032e-92bb-4579-aebf-311dab8e3184)
![CleanShot 2025-01-16 at 03 20
50](https://github.com/user-attachments/assets/9dd7b37c-8ced-4f50-b247-fdd7cd698247)

### **Before without "Unlimited" support**

<img width="320"
src="https://github.com/user-attachments/assets/cd0864e5-5497-4e5a-8507-3e5a2ef0b812">

### **After "Unlimited" support**

<img width="320"
src="https://github.com/user-attachments/assets/99d68e50-e381-42cd-b88f-724278fbd597">

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
  • Loading branch information
digiwand authored Jan 16, 2025
1 parent f27dbe6 commit d044c1c
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ const TypedSignV3V4Simulation: React.FC<object> = () => {
}

const { decodingData, decodingLoading } = signatureRequest;
const hasDecodingData = !(
const hasValidDecodingData = !(
(!decodingLoading && decodingData === undefined) ||
decodingData?.error
);

if (!hasDecodingData && isPermit) {
if (!hasValidDecodingData && isPermit) {
return <PermitSimulation />;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ describe('DecodedSimulation', () => {
expect(await getByText('12,345')).toBeDefined();
});

it('renders "Unlimited" for large values', async () => {
const { getByText } = renderWithProvider(<TypedSignDecoded />, {
state: mockState([{
...stateChangesApprove[0],
amount: '1461501637330902918203684832716283019655932542975',
}]),
});

expect(await getByText('Estimated changes')).toBeDefined();
expect(await getByText('Spending cap')).toBeDefined();
expect(await getByText('Unlimited')).toBeDefined();
});

it('renders for ERC712 token', async () => {
const { getByText } = renderWithProvider(<TypedSignDecoded />, {
state: mockState(stateChangesNftListing),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ const StateChangeRow = ({
stateChange;
const nftTransactionType = getStateChangeType(stateChangeList, stateChange);
const tooltip = shouldDisplayLabel ? getStateChangeToolip(nftTransactionType) : undefined;
// todo: add
// const canDisplayValueAsUnlimited =
// assetType === TokenStandard.ERC20 &&
// (changeType === DecodingDataChangeType.Approve ||
// changeType === DecodingDataChangeType.Revoke);

const canDisplayValueAsUnlimited =
assetType === TokenStandard.ERC20 &&
(changeType === DecodingDataChangeType.Approve ||
changeType === DecodingDataChangeType.Revoke);

const changeLabel = shouldDisplayLabel
? getStateChangeLabelMap(changeType, nftTransactionType)
Expand All @@ -133,8 +133,7 @@ const StateChangeRow = ({
changeType === DecodingDataChangeType.Receive
}
debit={changeType === DecodingDataChangeType.Transfer}
// todo: add
// canDisplayValueAsUnlimited={canDisplayValueAsUnlimited}
canDisplayValueAsUnlimited={canDisplayValueAsUnlimited}
/>
)}
{assetType === 'NATIVE' && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const mockTrackEvent = jest.fn();
jest.mock('../../../../../../../../../hooks/useMetrics');
jest.mock('../../../../../../../hooks/useGetTokenStandardAndDetails');


jest.mock('../../../../../../../../../../util/address', () => ({
getTokenDetails: jest.fn(),
renderShortAddress: jest.requireActual('../../../../../../../../../../util/address').renderShortAddress
Expand Down Expand Up @@ -72,11 +71,30 @@ describe('SimulationValueDisplay', () => {
{ state: mockInitialState },
);

await act(async () => {
await Promise.resolve();
expect(await findByText('0.432')).toBeDefined();
});

it('renders "Unlimited" for large values when canDisplayValueAsUnlimited is true', async () => {
(useGetTokenStandardAndDetails as jest.MockedFn<typeof useGetTokenStandardAndDetails>).mockReturnValue({
symbol: 'TST',
decimals: '4',
balance: undefined,
standard: TokenStandard.ERC20,
decimalsNumber: 4,
});

expect(await findByText('0.432')).toBeDefined();
const { findByText } = renderWithProvider(
<SimulationValueDisplay
canDisplayValueAsUnlimited
labelChangeType={'Spending Cap'}
tokenContract={'0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'}
value={'1461501637330902918203684832716283019655932542975'}
chainId={'0x1'}
/>,
{ state: mockInitialState },
);

expect(await findByText('Unlimited')).toBeDefined();
});

it('should invoke method to track missing decimal information for ERC20 tokens only once', async () => {
Expand All @@ -98,10 +116,6 @@ describe('SimulationValueDisplay', () => {
{ state: mockInitialState },
);

await act(async () => {
await Promise.resolve();
});

expect(mockTrackEvent).toHaveBeenCalledTimes(1);
});

Expand All @@ -124,10 +138,6 @@ describe('SimulationValueDisplay', () => {
{ state: mockInitialState },
);

await act(async () => {
await Promise.resolve();
});

expect(mockTrackEvent).not.toHaveBeenCalled();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ import { calcTokenAmount } from '../../../../../../../../../../util/transactions

import useGetTokenStandardAndDetails from '../../../../../../../hooks/useGetTokenStandardAndDetails';
import useTrackERC20WithoutDecimalInformation from '../../../../../../../hooks/useTrackERC20WithoutDecimalInformation';
import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../../../../../../utils/confirm';
import { TokenDetailsERC20 } from '../../../../../../../utils/token';
import BottomModal from '../../../../../../UI/BottomModal';

import styleSheet from './ValueDisplay.styles';
import { strings } from '../../../../../../../../../../../locales/i18n';

interface SimulationValueDisplayParams {
/** ID of the associated chain. */
Expand All @@ -53,6 +55,9 @@ interface SimulationValueDisplayParams {

// Optional

/** Whether a large amount can be substituted by "Unlimited" */
canDisplayValueAsUnlimited?: boolean;

/** True if value is being credited to wallet */
credit?: boolean;

Expand Down Expand Up @@ -81,6 +86,7 @@ const SimulationValueDisplay: React.FC<
value,
credit,
debit,
canDisplayValueAsUnlimited = false,
}) => {
const [hasValueModalOpen, setHasValueModalOpen] = useState(false);

Expand Down Expand Up @@ -112,6 +118,9 @@ const SimulationValueDisplay: React.FC<
const tokenValue = isValidTokenAmount ? formatAmount('en-US', tokenAmount) : null;
const tokenValueMaxPrecision = isValidTokenAmount ? formatAmountMaxPrecision('en-US', tokenAmount) : null;

const shouldShowUnlimitedValue = canDisplayValueAsUnlimited &&
Number(value) > TOKEN_VALUE_UNLIMITED_THRESHOLD;

/** Temporary error capturing as we are building out Permit Simulations */
if (!tokenContract) {
Logger.error(
Expand Down Expand Up @@ -140,8 +149,10 @@ const SimulationValueDisplay: React.FC<
<Text>
{credit && '+ '}
{debit && '- '}
{tokenValue !== null &&
shortenString(tokenValue || '', {
{shouldShowUnlimitedValue
? strings('confirm.unlimited')
: tokenValue !== null &&
shortenString(tokenValue || '', {
truncatedCharLimit: 15,
truncatedStartChars: 15,
truncatedEndChars: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const ExpandableSection = ({
</View>
</TouchableOpacity>
{expanded && (
<BottomModal hideBackground>
<BottomModal onClose={() => setExpanded(false)} canCloseOnBackdropClick>
<View style={styles.modalContent}>
<View style={styles.modalHeader}>
<ButtonIcon
Expand Down
3 changes: 3 additions & 0 deletions app/components/Views/confirmations/utils/confirm.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { ApprovalTypes } from '../../../../core/RPCMethods/RPCMethodMiddleware';

export const TOKEN_VALUE_UNLIMITED_THRESHOLD = 10 ** 15;

export function isSignatureRequest(requestType: string) {
return [
ApprovalTypes.PERSONAL_SIGN,
ApprovalTypes.ETH_SIGN_TYPED_DATA,
].includes(requestType as ApprovalTypes);
}

3 changes: 2 additions & 1 deletion locales/languages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -3622,7 +3622,8 @@
"title": "Estimated changes",
"tooltip": "Estimated changes are what might happen if you go through with this transaction. This is just a prediction, not a guarantee.",
"unavailable": "Unavailable"
}
},
"unlimited": "Unlimited"
},
"change_in_simulation_modal": {
"title": "Results have changed",
Expand Down

0 comments on commit d044c1c

Please sign in to comment.