Skip to content

Commit

Permalink
fix: Fix in label displayed for state change in signature decoding se…
Browse files Browse the repository at this point in the history
…ction (#29020)

## **Description**

We currently displaying multiple "You list" and "Spending cap" for
multiple assets. We should be displaying the copy only once similar to
how we do it for simulations.

## **Related issues**

Fixes: #28944

## **Manual testing steps**

1. Submit a typed sign v4 request with multiple NFT listed
2. Check simulation section on page displayed

## **Screenshots/Recordings**
<img width="359" alt="Screenshot 2024-12-09 at 6 05 46 PM"
src="https://github.com/user-attachments/assets/d847f04b-be71-49c2-a3f3-77fdc23bdddd">

## **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/main/.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/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
jpuri authored Dec 12, 2024
1 parent acdf7c6 commit 717cd87
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import configureMockStore from 'redux-mock-store';
import {
DecodingData,
DecodingDataChangeType,
DecodingDataStateChanges,
} from '@metamask/signature-controller';
Expand All @@ -11,7 +10,7 @@ import { renderWithConfirmContextProvider } from '../../../../../../../../../tes
import { permitSignatureMsg } from '../../../../../../../../../test/data/confirmations/typed_sign';
import PermitSimulation, { getStateChangeToolip } from './decoded-simulation';

const decodingData: DecodingData = {
const decodingData = {
stateChanges: [
{
assetType: 'ERC20',
Expand Down Expand Up @@ -165,4 +164,26 @@ describe('DecodedSimulation', () => {
);
expect(tooltip).toBe('signature_decoding_bid_nft_tooltip');
});

it('renders label only once if there are multiple state changes of same changeType', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData: {
stateChanges: [
decodingData.stateChanges[0],
decodingData.stateChanges[0],
decodingData.stateChanges[0],
],
},
});
const mockStore = configureMockStore([])(state);

const { findAllByText } = renderWithConfirmContextProvider(
<PermitSimulation />,
mockStore,
);

expect(await findAllByText('Spending cap')).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,20 @@ const StateChangeRow = ({
stateChangeList,
stateChange,
chainId,
shouldDisplayLabel,
}: {
stateChangeList: DecodingDataStateChanges | null;
stateChange: DecodingDataStateChange;
chainId: Hex;
shouldDisplayLabel: boolean;
}) => {
const t = useI18nContext();
const { assetType, changeType, amount, contractAddress, tokenID } =
stateChange;
const tooltip = getStateChangeToolip(stateChangeList, stateChange, t);
return (
<ConfirmInfoRow
label={getStateChangeLabelMap(t, changeType)}
label={shouldDisplayLabel ? getStateChangeLabelMap(t, changeType) : ''}
tooltip={tooltip}
>
{(assetType === TokenStandard.ERC20 ||
Expand Down Expand Up @@ -104,19 +106,31 @@ const DecodedSimulation: React.FC<object> = () => {
const chainId = currentConfirmation.chainId as Hex;
const { decodingLoading, decodingData } = currentConfirmation;

const stateChangeFragment = useMemo(
() =>
(decodingData?.stateChanges ?? []).map(
(change: DecodingDataStateChange) => (
<StateChangeRow
stateChangeList={decodingData?.stateChanges ?? []}
stateChange={change}
chainId={chainId}
/>
),
),
[decodingData?.stateChanges],
);
const stateChangeFragment = useMemo(() => {
const stateChangesGrouped: Record<string, DecodingDataStateChange[]> = (
decodingData?.stateChanges ?? []
).reduce<Record<string, DecodingDataStateChange[]>>(
(result, stateChange) => {
result[stateChange.changeType] = [
...(result[stateChange.changeType] ?? []),
stateChange,
];
return result;
},
{},
);

return Object.entries(stateChangesGrouped).flatMap(([_, changeList]) =>
changeList.map((change: DecodingDataStateChange, index: number) => (
<StateChangeRow
stateChangeList={decodingData?.stateChanges ?? []}
stateChange={change}
chainId={chainId}
shouldDisplayLabel={index === 0}
/>
)),
);
}, [decodingData?.stateChanges]);

return (
<StaticSimulation
Expand Down

0 comments on commit 717cd87

Please sign in to comment.