Skip to content

Commit 89f8021

Browse files
bergarcesbergarces
authored
fix: replicate network change actions in rpc modal (#29943)
<!-- 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? --> The issue occurred because switching networks from the RPC selector, which is only available when there's a network with at least 2 RPCs, was not performing the same actions as switching networks from clicking the network list item directly. I have created a handler for the actions that need to be performed and passed it down to both the network list item and the rpc selector modal, that way it ensures that the same actions occur. A test has been added to the rpc modal to validate the new handler is being called. Tests already exist in the network list to validate that those actions are taking place, and they are still passing. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29943?quickstart=1) ## **Related issues** Fixes: #29260 ## **Manual testing steps** 1. Ensure at least one of your networks has more than one RPC, so that the RPC selector appears. 2. Set any network and, in the Tokens tab, select "Current network" as the option. 3. Open the network selector and select another network by clicking in the rpc url, which will open an rpc selector modal. 4. Select any of the RPCs and check that the tokens from the new network appear and that "Current network" is still ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/3ac163d3-4f2e-4170-81f4-f3b4ccc8e3d3 ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/7dc02f85-42e0-449e-a419-cc74fc936de7 ## **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.
1 parent a9d90e5 commit 89f8021

File tree

3 files changed

+96
-76
lines changed

3 files changed

+96
-76
lines changed

‎ui/components/multichain/network-list-menu/network-list-menu.tsx

+49-44
Original file line numberDiff line numberDiff line change
@@ -252,17 +252,59 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => {
252252
...searchedTestNetworks,
253253
].some((network) => network.rpcEndpoints.length > 1);
254254

255+
const handleNetworkChange = (network: NetworkConfiguration) => {
256+
const allOpts = Object.keys(allNetworks).reduce((acc, chainId) => {
257+
acc[chainId] = true;
258+
return acc;
259+
}, {} as Record<string, boolean>);
260+
261+
const { networkClientId } =
262+
network.rpcEndpoints[network.defaultRpcEndpointIndex];
263+
dispatch(setActiveNetwork(networkClientId));
264+
dispatch(toggleNetworkMenu());
265+
dispatch(updateCustomNonce(''));
266+
dispatch(setNextNonce(''));
267+
dispatch(detectNfts());
268+
269+
// as a user, I don't want my network selection to force update my filter when I have "All Networks" toggled on
270+
// however, if I am already filtered on "Current Network", we'll want to filter by the selected network when the network changes
271+
if (Object.keys(tokenNetworkFilter || {}).length <= 1) {
272+
dispatch(setTokenNetworkFilter({ [network.chainId]: true }));
273+
} else if (process.env.PORTFOLIO_VIEW) {
274+
dispatch(setTokenNetworkFilter(allOpts));
275+
}
276+
277+
if (permittedAccountAddresses.length > 0) {
278+
dispatch(addPermittedChain(selectedTabOrigin, network.chainId));
279+
if (!permittedChainIds.includes(network.chainId)) {
280+
dispatch(showPermittedNetworkToast());
281+
}
282+
}
283+
// If presently on a dapp, communicate a change to
284+
// the dapp via silent switchEthereumChain that the
285+
// network has changed due to user action
286+
if (selectedTabOrigin && domains[selectedTabOrigin]) {
287+
setNetworkClientIdForDomain(selectedTabOrigin, networkClientId);
288+
}
289+
290+
trackEvent({
291+
event: MetaMetricsEventName.NavNetworkSwitched,
292+
category: MetaMetricsEventCategory.Network,
293+
properties: {
294+
location: 'Network Menu',
295+
chain_id: currentChainId,
296+
from_network: currentChainId,
297+
to_network: network.chainId,
298+
},
299+
});
300+
};
301+
255302
// Renders a network in the network list
256303
const generateNetworkListItem = (network: NetworkConfiguration) => {
257304
const isCurrentNetwork = network.chainId === currentChainId;
258305
const canDeleteNetwork =
259306
isUnlocked && !isCurrentNetwork && network.chainId !== CHAIN_IDS.MAINNET;
260307

261-
const allOpts: Record<string, boolean> = {};
262-
Object.keys(allNetworks).forEach((chainId) => {
263-
allOpts[chainId] = true;
264-
});
265-
266308
return (
267309
<NetworkListItem
268310
name={network.name}
@@ -282,45 +324,7 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => {
282324
selected={isCurrentNetwork && !focusSearch}
283325
focus={isCurrentNetwork && !focusSearch}
284326
onClick={() => {
285-
const { networkClientId } =
286-
network.rpcEndpoints[network.defaultRpcEndpointIndex];
287-
dispatch(setActiveNetwork(networkClientId));
288-
dispatch(toggleNetworkMenu());
289-
dispatch(updateCustomNonce(''));
290-
dispatch(setNextNonce(''));
291-
dispatch(detectNfts());
292-
293-
// as a user, I don't want my network selection to force update my filter when I have "All Networks" toggled on
294-
// however, if I am already filtered on "Current Network", we'll want to filter by the selected network when the network changes
295-
if (Object.keys(tokenNetworkFilter || {}).length <= 1) {
296-
dispatch(setTokenNetworkFilter({ [network.chainId]: true }));
297-
} else if (process.env.PORTFOLIO_VIEW) {
298-
dispatch(setTokenNetworkFilter(allOpts));
299-
}
300-
301-
if (permittedAccountAddresses.length > 0) {
302-
dispatch(addPermittedChain(selectedTabOrigin, network.chainId));
303-
if (!permittedChainIds.includes(network.chainId)) {
304-
dispatch(showPermittedNetworkToast());
305-
}
306-
}
307-
// If presently on a dapp, communicate a change to
308-
// the dapp via silent switchEthereumChain that the
309-
// network has changed due to user action
310-
if (selectedTabOrigin && domains[selectedTabOrigin]) {
311-
setNetworkClientIdForDomain(selectedTabOrigin, networkClientId);
312-
}
313-
314-
trackEvent({
315-
event: MetaMetricsEventName.NavNetworkSwitched,
316-
category: MetaMetricsEventCategory.Network,
317-
properties: {
318-
location: 'Network Menu',
319-
chain_id: currentChainId,
320-
from_network: currentChainId,
321-
to_network: network.chainId,
322-
},
323-
});
327+
handleNetworkChange(network);
324328
}}
325329
onDeleteClick={
326330
canDeleteNetwork
@@ -559,6 +563,7 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => {
559563
return (
560564
<SelectRpcUrlModal
561565
networkConfiguration={networkConfigurations[editedNetwork.chainId]}
566+
onNetworkChange={handleNetworkChange}
562567
/>
563568
);
564569
}

‎ui/components/multichain/network-list-menu/select-rpc-url-modal/select-rpc-url-modal.test.tsx

+38-18
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ jest.mock('react-redux', () => ({
1919
useDispatch: () => mockDispatch,
2020
}));
2121

22+
const mockOnNetworkChange = jest.fn();
23+
2224
jest.mock('../../../../store/actions', () => ({
2325
updateNetwork: jest.fn(),
2426
setActiveNetwork: jest.fn(),
@@ -51,15 +53,21 @@ describe('SelectRpcUrlModal Component', () => {
5153

5254
it('renders select rpc url', () => {
5355
const { container } = renderWithProvider(
54-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
56+
<SelectRpcUrlModal
57+
networkConfiguration={networkConfiguration}
58+
onNetworkChange={mockOnNetworkChange}
59+
/>,
5560
store,
5661
);
5762
expect(container).toMatchSnapshot();
5863
});
5964

6065
it('should render the component correctly with network image and name', () => {
6166
const { getByRole, getByText } = renderWithProvider(
62-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
67+
<SelectRpcUrlModal
68+
networkConfiguration={networkConfiguration}
69+
onNetworkChange={mockOnNetworkChange}
70+
/>,
6371
store,
6472
);
6573

@@ -77,7 +85,10 @@ describe('SelectRpcUrlModal Component', () => {
7785

7886
it('should render all RPC endpoints and highlight the selected one', () => {
7987
const { getByText } = renderWithProvider(
80-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
88+
<SelectRpcUrlModal
89+
networkConfiguration={networkConfiguration}
90+
onNetworkChange={mockOnNetworkChange}
91+
/>,
8192
store,
8293
);
8394

@@ -94,7 +105,10 @@ describe('SelectRpcUrlModal Component', () => {
94105

95106
it('should dispatch the correct actions when an RPC endpoint is clicked', () => {
96107
const { getByText } = renderWithProvider(
97-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
108+
<SelectRpcUrlModal
109+
networkConfiguration={networkConfiguration}
110+
onNetworkChange={mockOnNetworkChange}
111+
/>,
98112
store,
99113
);
100114

@@ -116,7 +130,10 @@ describe('SelectRpcUrlModal Component', () => {
116130

117131
it('should render the selected indicator correctly for the default RPC', () => {
118132
const { container } = renderWithProvider(
119-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
133+
<SelectRpcUrlModal
134+
networkConfiguration={networkConfiguration}
135+
onNetworkChange={mockOnNetworkChange}
136+
/>,
120137
store,
121138
);
122139

@@ -128,7 +145,10 @@ describe('SelectRpcUrlModal Component', () => {
128145

129146
it('should render the modal with a network image', () => {
130147
renderWithProvider(
131-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
148+
<SelectRpcUrlModal
149+
networkConfiguration={networkConfiguration}
150+
onNetworkChange={mockOnNetworkChange}
151+
/>,
132152
store,
133153
);
134154

@@ -142,26 +162,26 @@ describe('SelectRpcUrlModal Component', () => {
142162
);
143163
});
144164

145-
it('should handle click on RPC URL and update the network', () => {
165+
it('should handle click on RPC URL and call onNetworkChange', () => {
166+
const updatedNetwork = {
167+
...networkConfiguration,
168+
defaultRpcEndpointIndex: 1,
169+
};
170+
146171
renderWithProvider(
147-
<SelectRpcUrlModal networkConfiguration={networkConfiguration} />,
172+
<SelectRpcUrlModal
173+
networkConfiguration={networkConfiguration}
174+
onNetworkChange={mockOnNetworkChange}
175+
/>,
148176
store,
149177
);
150178

151179
fireEvent.click(
152180
screen.getByText(stripProtocol(networkConfiguration.rpcEndpoints[1].url)),
153181
);
154182

155-
expect(mockDispatch).toHaveBeenCalledWith(
156-
updateNetwork({
157-
...networkConfiguration,
158-
defaultRpcEndpointIndex: 1,
159-
}),
160-
);
161-
expect(mockDispatch).toHaveBeenCalledWith(
162-
setActiveNetwork(networkConfiguration.rpcEndpoints[1].networkClientId),
163-
);
183+
expect(mockDispatch).toHaveBeenCalledWith(updateNetwork(updatedNetwork));
164184
expect(mockDispatch).toHaveBeenCalledWith(setEditedNetwork());
165-
expect(mockDispatch).toHaveBeenCalledWith(toggleNetworkMenu());
185+
expect(mockOnNetworkChange).toHaveBeenCalledWith(updatedNetwork);
166186
});
167187
});

‎ui/components/multichain/network-list-menu/select-rpc-url-modal/select-rpc-url-modal.tsx

+9-14
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,15 @@ import {
1717
TextVariant,
1818
} from '../../../../helpers/constants/design-system';
1919
import { CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP } from '../../../../../shared/constants/network';
20-
import {
21-
setActiveNetwork,
22-
setEditedNetwork,
23-
toggleNetworkMenu,
24-
updateNetwork,
25-
} from '../../../../store/actions';
20+
import { setEditedNetwork, updateNetwork } from '../../../../store/actions';
2621
import RpcListItem from '../rpc-list-item';
2722

2823
export const SelectRpcUrlModal = ({
2924
networkConfiguration,
25+
onNetworkChange,
3026
}: {
3127
networkConfiguration: NetworkConfiguration;
28+
onNetworkChange: (network: NetworkConfiguration) => void;
3229
}) => {
3330
const dispatch = useDispatch();
3431

@@ -69,15 +66,13 @@ export const SelectRpcUrlModal = ({
6966
display={Display.Flex}
7067
key={rpcEndpoint.url}
7168
onClick={() => {
72-
dispatch(
73-
updateNetwork({
74-
...networkConfiguration,
75-
defaultRpcEndpointIndex: index,
76-
}),
77-
);
78-
dispatch(setActiveNetwork(rpcEndpoint.networkClientId));
69+
const network = {
70+
...networkConfiguration,
71+
defaultRpcEndpointIndex: index,
72+
};
73+
dispatch(updateNetwork(network));
7974
dispatch(setEditedNetwork());
80-
dispatch(toggleNetworkMenu());
75+
onNetworkChange(network);
8176
}}
8277
className={classnames('select-rpc-url__item', {
8378
'select-rpc-url__item--selected':

0 commit comments

Comments
 (0)