Skip to content

Commit 09d0237

Browse files
authored
fix: cp-7.51.0 cp-7.52.0 Fix TokenHero component and put back e2e assertions (#17403)
<!-- 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 aims to fix broken `TokenHero` component for native assets. Please see recording that all type of assets are shown properly in `TokenHero` component now. It also puts back e2e assertions in place. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: MetaMask/MetaMask-planning#5403 ## **Manual testing steps** 1. Try sending ETH 2. See correct amount shown in the confirmation ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/e5c5aebc-8f6e-4963-9b97-452e3df7aa28 ## **Pre-merge author checklist** - [X] 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). - [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-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.
1 parent 205fff0 commit 09d0237

File tree

7 files changed

+94
-29
lines changed

7 files changed

+94
-29
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { StyleSheet } from 'react-native';
2+
import { Theme } from '../../../../../../../util/theme/models';
3+
4+
const styleSheet = (params: { theme: Theme }) => {
5+
const { theme } = params;
6+
7+
return StyleSheet.create({
8+
wrapper: {
9+
minHeight: 100,
10+
justifyContent: 'center',
11+
alignItems: 'center',
12+
},
13+
loadingWrapper: {
14+
height: 78,
15+
width: 78,
16+
borderRadius: 39,
17+
backgroundColor: theme.colors.background.alternativePressed,
18+
},
19+
});
20+
};
21+
22+
export default styleSheet;

app/components/Views/confirmations/components/rows/transactions/hero-row/hero-row.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,26 @@ import { ConfirmationRowComponentIDs } from '../../../../../../../../e2e/selecto
55
import { useIsNft } from '../../../../hooks/nft/useIsNft';
66
import { HeroNft } from '../../../hero-nft';
77
import { HeroToken } from '../../../hero-token';
8+
import { useStyles } from '../../../../../../../component-library/hooks';
9+
import styleSheet from './hero-row.styles';
810

9-
const styles = {
10-
wrapper: {
11-
minHeight: 100,
12-
},
11+
const LoadingHeroRow = () => {
12+
const { styles } = useStyles(styleSheet, {});
13+
return <View style={styles.loadingWrapper} />;
1314
};
1415

1516
export const HeroRow = ({ amountWei }: { amountWei?: string }) => {
1617
const { isNft, isPending } = useIsNft();
18+
const { styles } = useStyles(styleSheet, {});
1719

1820
return (
1921
<View
2022
style={styles.wrapper}
2123
testID={ConfirmationRowComponentIDs.TOKEN_HERO}
2224
>
23-
{!isPending && isNft === true && <HeroNft />}
24-
{!isPending && isNft === false && <HeroToken amountWei={amountWei} />}
25+
{isPending && <LoadingHeroRow />}
26+
{!isPending &&
27+
(isNft ? <HeroNft /> : <HeroToken amountWei={amountWei} />)}
2528
</View>
2629
);
2730
};

app/components/Views/confirmations/hooks/nft/useIsNft.test.ts

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { useIsNft } from './useIsNft';
21
import { renderHookWithProvider } from '../../../../../util/test/renderWithProvider';
32
import { backgroundState } from '../../../../../util/test/initial-root-state';
3+
import { TokenStandard } from '../../../../UI/SimulationDetails/types';
44
import { useGetTokenStandardAndDetails } from '../useGetTokenStandardAndDetails';
5+
import { useIsNft } from './useIsNft';
56

67
jest.mock('../transactions/useTransactionMetadataRequest', () => ({
78
useTransactionMetadataRequest: jest.fn().mockReturnValue({
@@ -25,7 +26,41 @@ describe('useIsNft', () => {
2526
jest.clearAllMocks();
2627
});
2728

28-
it('returns isNft true when token standard is not ERC20', () => {
29+
it.each([
30+
{ standard: TokenStandard.ERC1155, isNft: true },
31+
{ standard: TokenStandard.ERC721, isNft: true },
32+
{ standard: TokenStandard.ERC20, isNft: false },
33+
])(
34+
'returns isNft $isNft when token standard is $standard',
35+
({ standard, isNft }) => {
36+
(useGetTokenStandardAndDetails as jest.Mock).mockReturnValue({
37+
details: {
38+
standard,
39+
},
40+
isPending: false,
41+
});
42+
43+
const { result } = renderHookWithProvider(useIsNft, {
44+
state: {
45+
engine: {
46+
backgroundState,
47+
},
48+
},
49+
});
50+
51+
expect(result.current).toEqual({
52+
isNft,
53+
isPending: false,
54+
});
55+
},
56+
);
57+
58+
it('returns undefined when token standard is undefined', () => {
59+
(useGetTokenStandardAndDetails as jest.Mock).mockReturnValue({
60+
details: undefined,
61+
isPending: true,
62+
});
63+
2964
const { result } = renderHookWithProvider(useIsNft, {
3065
state: {
3166
engine: {
@@ -35,17 +70,15 @@ describe('useIsNft', () => {
3570
});
3671

3772
expect(result.current).toEqual({
38-
isNft: true,
39-
isPending: false,
73+
isNft: undefined,
74+
isPending: true,
4075
});
4176
});
4277

43-
it('returns isNft false when token standard is ERC20', () => {
78+
it('when token standard is on pending state, returns undefined', () => {
4479
(useGetTokenStandardAndDetails as jest.Mock).mockReturnValue({
45-
details: {
46-
standard: 'ERC20',
47-
},
48-
isPending: false,
80+
details: undefined,
81+
isPending: true,
4982
});
5083

5184
const { result } = renderHookWithProvider(useIsNft, {
@@ -57,8 +90,8 @@ describe('useIsNft', () => {
5790
});
5891

5992
expect(result.current).toEqual({
60-
isNft: false,
61-
isPending: false,
93+
isNft: undefined,
94+
isPending: true,
6295
});
6396
});
6497
});

app/components/Views/confirmations/hooks/nft/useIsNft.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@ export const useIsNft = (): { isNft?: boolean; isPending: boolean } => {
1414
networkClientId,
1515
);
1616

17+
// Native token / loading state
18+
if (isPending || details?.standard === undefined) {
19+
return { isNft: undefined, isPending };
20+
}
21+
22+
// NFT check
1723
const isNft =
18-
!isPending && details.standard !== undefined
19-
? details.standard !== TokenStandard.ERC20
20-
: undefined;
24+
details.standard === TokenStandard.ERC1155 ||
25+
details.standard === TokenStandard.ERC721;
26+
2127
return { isNft, isPending };
2228
};

app/components/Views/confirmations/legacy/SendFlow/Amount/index.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,9 +635,15 @@ class Amount extends PureComponent {
635635
selectedAsset: { address, tokenId },
636636
},
637637
selectedAddress,
638+
globalNetworkClientId,
638639
} = this.props;
639640
try {
640-
return await NftController.isNftOwner(selectedAddress, address, tokenId);
641+
return await NftController.isNftOwner(
642+
selectedAddress,
643+
address,
644+
tokenId,
645+
globalNetworkClientId,
646+
);
641647
} catch (e) {
642648
return false;
643649
}

e2e/specs/confirmations-redesigned/transactions/send-max-transfer.spec.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@ import {
1111
SIMULATION_ENABLED_NETWORKS_MOCK,
1212
} from '../../../api-mocking/mock-responses/simulations';
1313
import Assertions from '../../../utils/Assertions';
14-
import ConfirmationUITypes from '../../../pages/Browser/Confirmations/ConfirmationUITypes';
1514
import WalletActionsBottomSheet from '../../../pages/wallet/WalletActionsBottomSheet';
1615
import FixtureBuilder from '../../../fixtures/fixture-builder';
1716
import { mockEvents } from '../../../api-mocking/mock-config/mock-events';
1817
import TabBarComponent from '../../../pages/wallet/TabBarComponent';
1918
import FooterActions from '../../../pages/Browser/Confirmations/FooterActions';
2019
import SendView from '../../../pages/Send/SendView';
2120
import AmountView from '../../../pages/Send/AmountView';
22-
import RowComponents from '../../../pages/Browser/Confirmations/RowComponents';
2321

2422
const RECIPIENT = '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb';
2523

@@ -68,12 +66,7 @@ describe(SmokeConfirmationsRedesigned('Send Max Transfer'), () => {
6866
await AmountView.tapNextButton();
6967

7068
// Check all expected elements are visible
71-
await Assertions.checkIfVisible(
72-
ConfirmationUITypes.FlatConfirmationContainer,
73-
);
74-
await Assertions.checkIfVisible(RowComponents.FromTo);
75-
await Assertions.checkIfVisible(RowComponents.GasFeesDetails);
76-
await Assertions.checkIfVisible(RowComponents.AdvancedDetails);
69+
await Assertions.checkIfTextIsDisplayed('1,000 ETH');
7770

7871
// Accept confirmation
7972
await FooterActions.tapConfirmButton();

e2e/specs/confirmations-redesigned/transactions/wallet-initiated-transfer.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ describe(SmokeConfirmationsRedesigned('Wallet Initiated Transfer'), () => {
6868
await Assertions.checkIfVisible(
6969
ConfirmationUITypes.FlatConfirmationContainer,
7070
);
71+
await Assertions.checkIfVisible(RowComponents.TokenHero);
72+
await Assertions.checkIfTextIsDisplayed('1 ETH');
7173
await Assertions.checkIfVisible(RowComponents.FromTo);
7274
await Assertions.checkIfVisible(RowComponents.GasFeesDetails);
7375
await Assertions.checkIfVisible(RowComponents.AdvancedDetails);

0 commit comments

Comments
 (0)