Skip to content

Commit df8313f

Browse files
salimtbsalimtb
authored
fix: when Network manager selection is Solana and a new popular network is added, the defi and NFTs tabs are missing (#38419)
<!-- 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** when Network manager selection is Solana and a new popular network is added, the defi and NFTs tabs are missing <!-- 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/38419?quickstart=1) ## **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: when Network manager selection is Solana and a new popular network is added, the defi and NFTs tabs are missing ## **Related issues** Fixes: ## **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** https://github.com/user-attachments/assets/573b2b55-3330-45e0-97ad-7dfd2ec686d2 <!-- [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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Non‑EVM account overview now shows NFTs and DeFi tabs only when any EVM network is enabled (and DeFi flag), powered by a new selector and comprehensive tests. > > - **UI (Account Overview)**: > - `AccountOverviewNonEvm` uses `getHasAnyEvmNetworkEnabled` and `getIsDefiPositionsEnabled` to set `showNfts` and `showDefi` in `AccountOverviewLayout`. > - **Selectors**: > - Add `getHasAnyEvmNetworkEnabled` to detect if any EVM network is enabled via `enabledNetworkMap`. > - **Tests**: > - Expand `account-overview-non-evm.test.tsx` to cover cases with no EVM networks, with EVM networks (DeFi on/off), single EVM network, and switching to non‑EVM (Solana). > - Add unit tests for `getHasAnyEvmNetworkEnabled` in `selectors.test.js`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f468f23. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent bfed73b commit df8313f

File tree

4 files changed

+240
-16
lines changed

4 files changed

+240
-16
lines changed

‎ui/components/multichain/account-overview/account-overview-non-evm.test.tsx‎

Lines changed: 128 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ const defaultProps: AccountOverviewNonEvmProps = {
3939
onSupportLinkClick: jest.fn(),
4040
};
4141

42-
const render = (props: AccountOverviewNonEvmProps = defaultProps) => {
42+
const render = (
43+
props: AccountOverviewNonEvmProps = defaultProps,
44+
stateOverrides = {},
45+
) => {
4346
const store = configureStore({
4447
metamask: {
4548
...mockState.metamask,
@@ -50,6 +53,10 @@ const render = (props: AccountOverviewNonEvmProps = defaultProps) => {
5053
[CHAIN_IDS.LINEA_MAINNET]: true,
5154
},
5255
},
56+
enabledNetworkMap: {
57+
eip155: {},
58+
},
59+
...stateOverrides,
5360
},
5461
});
5562

@@ -63,23 +70,129 @@ describe('AccountOverviewBtc', () => {
6370
} as never);
6471
});
6572

66-
it('shows only Tokens and Activity tabs', () => {
67-
const { queryByTestId } = render();
73+
describe('when no EVM networks are enabled', () => {
74+
it('shows only Tokens and Activity tabs', () => {
75+
const { queryByTestId } = render(defaultProps, {
76+
enabledNetworkMap: {
77+
eip155: {},
78+
},
79+
});
80+
81+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
82+
expect(
83+
queryByTestId('account-overview__nfts-tab'),
84+
).not.toBeInTheDocument();
85+
expect(
86+
queryByTestId('account-overview__activity-tab'),
87+
).toBeInTheDocument();
88+
expect(
89+
queryByTestId('account-overview__defi-tab'),
90+
).not.toBeInTheDocument();
91+
});
92+
93+
it('shows tokens links', () => {
94+
const { queryByTestId } = render(defaultProps, {
95+
enabledNetworkMap: {
96+
eip155: {},
97+
},
98+
});
99+
100+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
101+
const button = queryByTestId('asset-list-control-bar-action-button');
102+
expect(button).toBeInTheDocument(); // Verify the button is present
103+
expect(button).toBeEnabled(); // Verify the button is enabled
104+
// TODO: This one might be required, but we do not really handle tokens for BTC yet...
105+
expect(queryByTestId('refresh-list-button')).not.toBeInTheDocument();
106+
});
107+
});
108+
109+
describe('when EVM networks are enabled', () => {
110+
it('shows Tokens, NFTs, and Activity tabs when DeFi is disabled', () => {
111+
const { queryByTestId } = render(defaultProps, {
112+
enabledNetworkMap: {
113+
eip155: {
114+
[CHAIN_IDS.MAINNET]: true,
115+
[CHAIN_IDS.LINEA_MAINNET]: true,
116+
},
117+
},
118+
remoteFeatureFlags: {
119+
assetsDefiPositionsEnabled: false,
120+
},
121+
});
122+
123+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
124+
expect(queryByTestId('account-overview__nfts-tab')).toBeInTheDocument();
125+
expect(
126+
queryByTestId('account-overview__activity-tab'),
127+
).toBeInTheDocument();
128+
expect(
129+
queryByTestId('account-overview__defi-tab'),
130+
).not.toBeInTheDocument();
131+
});
132+
133+
it('shows all tabs including DeFi when DeFi positions are enabled', () => {
134+
const { queryByTestId } = render(defaultProps, {
135+
enabledNetworkMap: {
136+
eip155: {
137+
[CHAIN_IDS.MAINNET]: true,
138+
[CHAIN_IDS.LINEA_MAINNET]: true,
139+
},
140+
},
141+
remoteFeatureFlags: {
142+
assetsDefiPositionsEnabled: true,
143+
},
144+
});
68145

69-
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
70-
expect(queryByTestId('account-overview__nfts-tab')).not.toBeInTheDocument();
71-
expect(queryByTestId('account-overview__activity-tab')).toBeInTheDocument();
72-
expect(queryByTestId('account-overview__defi-tab')).not.toBeInTheDocument();
146+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
147+
expect(queryByTestId('account-overview__nfts-tab')).toBeInTheDocument();
148+
expect(
149+
queryByTestId('account-overview__activity-tab'),
150+
).toBeInTheDocument();
151+
expect(queryByTestId('account-overview__defi-tab')).toBeInTheDocument();
152+
});
153+
154+
it('shows NFTs and Activity tabs even when only one EVM network is enabled', () => {
155+
const { queryByTestId } = render(defaultProps, {
156+
enabledNetworkMap: {
157+
eip155: {
158+
[CHAIN_IDS.MAINNET]: true,
159+
[CHAIN_IDS.LINEA_MAINNET]: false,
160+
},
161+
},
162+
});
163+
164+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
165+
expect(queryByTestId('account-overview__nfts-tab')).toBeInTheDocument();
166+
expect(
167+
queryByTestId('account-overview__activity-tab'),
168+
).toBeInTheDocument();
169+
});
73170
});
74171

75-
it('shows tokens links', () => {
76-
const { queryByTestId } = render();
172+
describe('when switching between network types', () => {
173+
it('hides NFTs and DeFi tabs when all EVM networks are disabled', () => {
174+
const { queryByTestId } = render(defaultProps, {
175+
enabledNetworkMap: {
176+
eip155: {
177+
[CHAIN_IDS.MAINNET]: false,
178+
[CHAIN_IDS.LINEA_MAINNET]: false,
179+
},
180+
solana: {
181+
'solana:mainnet': true,
182+
},
183+
},
184+
});
77185

78-
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
79-
const button = queryByTestId('asset-list-control-bar-action-button');
80-
expect(button).toBeInTheDocument(); // Verify the button is present
81-
expect(button).toBeEnabled(); // Verify the button is enabled
82-
// TODO: This one might be required, but we do not really handle tokens for BTC yet...
83-
expect(queryByTestId('refresh-list-button')).not.toBeInTheDocument();
186+
expect(queryByTestId('account-overview__asset-tab')).toBeInTheDocument();
187+
expect(
188+
queryByTestId('account-overview__nfts-tab'),
189+
).not.toBeInTheDocument();
190+
expect(
191+
queryByTestId('account-overview__activity-tab'),
192+
).toBeInTheDocument();
193+
expect(
194+
queryByTestId('account-overview__defi-tab'),
195+
).not.toBeInTheDocument();
196+
});
84197
});
85198
});

‎ui/components/multichain/account-overview/account-overview-non-evm.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import React from 'react';
2+
import { useSelector } from 'react-redux';
23
import { NonEvmOverview } from '../../app/wallet-overview';
4+
import {
5+
getIsDefiPositionsEnabled,
6+
getHasAnyEvmNetworkEnabled,
7+
} from '../../../selectors';
38
import { AccountOverviewLayout } from './account-overview-layout';
49
import { AccountOverviewCommonProps } from './common';
510

@@ -8,11 +13,15 @@ export type AccountOverviewNonEvmProps = AccountOverviewCommonProps;
813
export const AccountOverviewNonEvm = ({
914
...props
1015
}: AccountOverviewNonEvmProps) => {
16+
const defiPositionsEnabled = useSelector(getIsDefiPositionsEnabled);
17+
const hasAnyEvmNetworkEnabled = useSelector(getHasAnyEvmNetworkEnabled);
18+
1119
return (
1220
<AccountOverviewLayout
1321
showTokens={true}
1422
showTokensLinks={true}
15-
showNfts={false}
23+
showNfts={hasAnyEvmNetworkEnabled}
24+
showDefi={hasAnyEvmNetworkEnabled && defiPositionsEnabled}
1625
showActivity={true}
1726
{...props}
1827
>

‎ui/selectors/selectors.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3294,6 +3294,20 @@ export function getIsDefiPositionsEnabled(state) {
32943294
return Boolean(assetsDefiPositionsEnabled);
32953295
}
32963296

3297+
/**
3298+
* Returns true if any EVM networks are enabled in the network filter.
3299+
*
3300+
* @param {*} state
3301+
* @returns {boolean} True if at least one EVM network is enabled.
3302+
*/
3303+
export function getHasAnyEvmNetworkEnabled(state) {
3304+
const enabledNetworks = getEnabledNetworks(state);
3305+
const enabledEvmNetworks = enabledNetworks[KnownCaipNamespace.Eip155] ?? {};
3306+
return Object.values(enabledEvmNetworks).some(
3307+
(isEnabled) => isEnabled === true,
3308+
);
3309+
}
3310+
32973311
export function getIsCustomNetwork(state) {
32983312
const chainId = getCurrentChainId(state);
32993313

‎ui/selectors/selectors.test.js‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ApprovalType } from '@metamask/controller-utils';
2+
import { KnownCaipNamespace } from '@metamask/utils';
23
import {
34
BtcAccountType,
45
EthAccountType,
@@ -3037,3 +3038,90 @@ describe('getGasFeesSponsoredNetworkEnabled', () => {
30373038
expect(result['0x2']).toBe(false);
30383039
});
30393040
});
3041+
3042+
describe('getHasAnyEvmNetworkEnabled', () => {
3043+
it('returns true when at least one EVM network is enabled', () => {
3044+
const state = {
3045+
metamask: {
3046+
enabledNetworkMap: {
3047+
[KnownCaipNamespace.Eip155]: {
3048+
'0x1': true,
3049+
'0x5': false,
3050+
},
3051+
},
3052+
},
3053+
};
3054+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(true);
3055+
});
3056+
3057+
it('returns false when no EVM networks are enabled', () => {
3058+
const state = {
3059+
metamask: {
3060+
enabledNetworkMap: {
3061+
[KnownCaipNamespace.Eip155]: {
3062+
'0x1': false,
3063+
'0x5': false,
3064+
},
3065+
},
3066+
},
3067+
};
3068+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(false);
3069+
});
3070+
3071+
it('returns false when EVM namespace is empty', () => {
3072+
const state = {
3073+
metamask: {
3074+
enabledNetworkMap: {
3075+
[KnownCaipNamespace.Eip155]: {},
3076+
},
3077+
},
3078+
};
3079+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(false);
3080+
});
3081+
3082+
it('returns false when EVM namespace is not present', () => {
3083+
const state = {
3084+
metamask: {
3085+
enabledNetworkMap: {
3086+
[KnownCaipNamespace.Solana]: {
3087+
'solana:mainnet': true,
3088+
},
3089+
},
3090+
},
3091+
};
3092+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(false);
3093+
});
3094+
3095+
it('returns true when multiple EVM networks are enabled', () => {
3096+
const state = {
3097+
metamask: {
3098+
enabledNetworkMap: {
3099+
[KnownCaipNamespace.Eip155]: {
3100+
'0x1': true,
3101+
'0x89': true,
3102+
'0xa': true,
3103+
},
3104+
},
3105+
},
3106+
};
3107+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(true);
3108+
});
3109+
3110+
it('returns true when mixed enabled/disabled EVM networks with at least one enabled', () => {
3111+
const state = {
3112+
metamask: {
3113+
enabledNetworkMap: {
3114+
[KnownCaipNamespace.Eip155]: {
3115+
'0x1': false,
3116+
'0x89': true,
3117+
'0xa': false,
3118+
},
3119+
[KnownCaipNamespace.Solana]: {
3120+
'solana:mainnet': true,
3121+
},
3122+
},
3123+
},
3124+
};
3125+
expect(selectors.getHasAnyEvmNetworkEnabled(state)).toBe(true);
3126+
});
3127+
});

0 commit comments

Comments
 (0)