Skip to content

Commit e784dfc

Browse files
fix: Gas station improvements (#37352)
<!-- 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? --> Fixes the icons and the auto selection logic for the gas station modal. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37352?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: ## **Related issues** Fixes: #37120 ## **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** <!-- [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] > Improve gas fee token icon rendering and refine auto-selection logic with expanded async tests; increase e2e toast close timeout. > > - **UI**: > - `ui/pages/.../gas-fee-token-icon.tsx`: Display ERC-20 token icons from store (`selectERC20TokensByChain` `iconUrl`) when available; fallback to `PreferredAvatar`; keep native token rendering via `AvatarToken`; size mapping via `AvatarAccountSize`. > - **Logic**: > - `useAutomaticGasFeeTokenSelect`: Only set `firstCheck` to `false` after successfully selecting the first eligible token; supports selection on rerender when eligibility changes (insufficient balance, gasless support, 7702/native handling). > - **Tests**: > - `useAutomaticGasFeeTokenSelect.test.ts`: Convert to async with `flushPromises`, add assertions for `forceUpdateMetamaskState`, cover rerender/eligibility and 7702/native scenarios. > - **E2E**: > - `transaction-confirmation.ts`: Increase toast close `clickElementSafe` timeout to `10000` for stability. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1b1e382. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0181bdf commit e784dfc

File tree

4 files changed

+164
-30
lines changed

4 files changed

+164
-30
lines changed

test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ class TransactionConfirmation extends Confirmation {
309309

310310
async closeGasFeeToastMessage() {
311311
// the toast message automatically disappears after some seconds, so we need to use clickElementSafe to prevent race conditions
312-
await this.driver.clickElementSafe(this.gasFeeCloseToastMessage, 5000);
312+
await this.driver.clickElementSafe(this.gasFeeCloseToastMessage, 10000);
313313
}
314314

315315
/**

ui/pages/confirmations/components/confirm/info/shared/gas-fee-token-icon/gas-fee-token-icon.tsx

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
1-
import React from 'react';
2-
import { Hex } from '@metamask/utils';
1+
import { AvatarAccountSize } from '@metamask/design-system-react';
32
import { TransactionMeta } from '@metamask/transaction-controller';
3+
import { Hex } from '@metamask/utils';
4+
import React from 'react';
45
import { useSelector } from 'react-redux';
5-
6-
import { NATIVE_TOKEN_ADDRESS } from '../../../../../../../../shared/constants/transaction';
7-
import { useConfirmContext } from '../../../../../context/confirm';
8-
import { selectNetworkConfigurationByChainId } from '../../../../../../../selectors';
9-
import Identicon from '../../../../../../../components/ui/identicon';
106
import { CHAIN_ID_TOKEN_IMAGE_MAP } from '../../../../../../../../shared/constants/network';
7+
import { NATIVE_TOKEN_ADDRESS } from '../../../../../../../../shared/constants/transaction';
8+
import { PreferredAvatar } from '../../../../../../../components/app/preferred-avatar';
119
import {
1210
AvatarToken,
1311
AvatarTokenSize,
1412
Box,
1513
} from '../../../../../../../components/component-library';
14+
import Identicon from '../../../../../../../components/ui/identicon';
1615
import { BackgroundColor } from '../../../../../../../helpers/constants/design-system';
16+
import {
17+
selectERC20TokensByChain,
18+
selectNetworkConfigurationByChainId,
19+
} from '../../../../../../../selectors';
20+
import { useConfirmContext } from '../../../../../context/confirm';
1721

1822
export enum GasFeeTokenIconSize {
1923
Sm = 'sm',
@@ -36,13 +40,30 @@ export function GasFeeTokenIcon({
3640
selectNetworkConfigurationByChainId(state, chainId),
3741
);
3842

43+
const erc20TokensByChain = useSelector(selectERC20TokensByChain);
44+
const variation = chainId;
45+
const { iconUrl: image } =
46+
erc20TokensByChain?.[variation]?.data?.[tokenAddress] ?? {};
47+
3948
if (tokenAddress !== NATIVE_TOKEN_ADDRESS) {
4049
return (
4150
<Box data-testid="token-icon">
42-
<Identicon
43-
address={tokenAddress}
44-
diameter={size === GasFeeTokenIconSize.Md ? 32 : 12}
45-
/>
51+
{image ? (
52+
<Identicon
53+
address={tokenAddress}
54+
diameter={size === GasFeeTokenIconSize.Md ? 32 : 12}
55+
image={image}
56+
/>
57+
) : (
58+
<PreferredAvatar
59+
address={tokenAddress}
60+
size={
61+
size === GasFeeTokenIconSize.Md
62+
? AvatarAccountSize.Md
63+
: AvatarAccountSize.Xs
64+
}
65+
/>
66+
)}
4667
</Box>
4768
);
4869
}

ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts

Lines changed: 130 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { NATIVE_TOKEN_ADDRESS } from '../../../../shared/constants/transaction';
66
import { genUnapprovedContractInteractionConfirmation } from '../../../../test/data/confirmations/contract-interaction';
77
import { getMockConfirmStateForTransaction } from '../../../../test/data/confirmations/helper';
88
import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers';
9+
import { flushPromises } from '../../../../test/lib/timer-helpers';
910
import { updateSelectedGasFeeToken } from '../../../store/controller-actions/transaction-controller';
1011
import { Alert } from '../../../ducks/confirm-alerts/confirm-alerts';
1112
import { forceUpdateMetamaskState } from '../../../store/actions';
@@ -46,6 +47,12 @@ function runHook({
4647
return { ...result, state };
4748
}
4849

50+
async function flushAsyncUpdates() {
51+
await act(async () => {
52+
await flushPromises();
53+
});
54+
}
55+
4956
describe('useAutomaticGasFeeTokenSelect', () => {
5057
const updateSelectedGasFeeTokenMock = jest.mocked(updateSelectedGasFeeToken);
5158
const forceUpdateMetamaskStateMock = jest.mocked(forceUpdateMetamaskState);
@@ -67,31 +74,56 @@ describe('useAutomaticGasFeeTokenSelect', () => {
6774
});
6875
});
6976

70-
it('selects first gas fee token', () => {
71-
runHook();
77+
it('selects first gas fee token', async () => {
78+
const { store } = runHook();
79+
80+
await flushAsyncUpdates();
81+
82+
if (!store) {
83+
throw new Error('Expected store to be defined');
84+
}
7285

7386
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1);
7487
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith(
7588
expect.any(String),
7689
GAS_FEE_TOKEN_MOCK.tokenAddress,
7790
);
91+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1);
92+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch);
7893
});
7994

80-
it('does not select first gas fee token if gas fee token already selected', () => {
95+
it('does not select first gas fee token if gas fee token already selected', async () => {
8196
runHook({ selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress });
97+
98+
await flushAsyncUpdates();
99+
82100
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
101+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
83102
});
84103

85-
it('does not select first gas fee token if no gas fee tokens', () => {
104+
it('does not select first gas fee token if no gas fee tokens', async () => {
86105
runHook({ gasFeeTokens: [] });
106+
107+
await flushAsyncUpdates();
108+
87109
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
110+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
88111
});
89112

90-
it('does not select first gas fee token if not first load', () => {
91-
const { rerender, state } = runHook({
113+
it('selects first gas fee token on rerender when selection becomes eligible', async () => {
114+
const { rerender, state, store } = runHook({
92115
selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress,
93116
});
94117

118+
if (!store) {
119+
throw new Error('Expected store to be defined');
120+
}
121+
122+
await flushAsyncUpdates();
123+
124+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
125+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
126+
95127
const transactionMeta = state.metamask
96128
.transactions[0] as unknown as TransactionMeta;
97129

@@ -101,41 +133,100 @@ describe('useAutomaticGasFeeTokenSelect', () => {
101133

102134
rerender();
103135

104-
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
136+
await flushAsyncUpdates();
137+
138+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1);
139+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith(
140+
expect.any(String),
141+
GAS_FEE_TOKEN_MOCK.tokenAddress,
142+
);
143+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1);
144+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch);
105145
});
106146

107-
it('does not select first gas fee token if gasless not supported', () => {
147+
it('does not select first gas fee token if gasless not supported', async () => {
108148
useIsGaslessSupportedMock.mockReturnValue({
109149
isSupported: false,
110150
isSmartTransaction: false,
111151
});
112152

113153
runHook();
114154

155+
await flushAsyncUpdates();
156+
115157
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
158+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
116159
});
117160

118-
it('does not select first gas fee token if sufficient balance', () => {
161+
it('does not select first gas fee token if sufficient balance', async () => {
119162
useInsufficientBalanceAlertsMock.mockReturnValue([]);
120163

121164
runHook();
122165

166+
await flushAsyncUpdates();
167+
123168
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
169+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
124170
});
125171

126-
it('does not select first gas fee token after firstCheck is set to false', () => {
127-
const { rerender, state } = runHook();
172+
it('selects first gas fee token when insufficient balance appears after first render', async () => {
173+
let alerts: Alert[] = [];
174+
useInsufficientBalanceAlertsMock.mockImplementation(() => alerts);
175+
176+
const { rerender, store } = runHook({
177+
selectedGasFeeToken: undefined,
178+
});
179+
180+
if (!store) {
181+
throw new Error('Expected store to be defined');
182+
}
183+
184+
await flushAsyncUpdates();
185+
186+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
187+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
188+
189+
alerts = [{} as Alert];
190+
191+
rerender();
192+
193+
await flushAsyncUpdates();
194+
195+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1);
196+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith(
197+
expect.any(String),
198+
GAS_FEE_TOKEN_MOCK.tokenAddress,
199+
);
200+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1);
201+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch);
202+
});
203+
204+
it('does not select first gas fee token after firstCheck is set to false', async () => {
205+
const { rerender, state, store } = runHook();
206+
207+
if (!store) {
208+
throw new Error('Expected store to be defined');
209+
}
210+
211+
await flushAsyncUpdates();
212+
128213
// Simulate a rerender with new state that would otherwise trigger selection
129214
act(() => {
130215
(
131216
state.metamask.transactions[0] as unknown as TransactionMeta
132217
).selectedGasFeeToken = undefined;
133218
});
219+
134220
rerender();
221+
222+
await flushAsyncUpdates();
223+
135224
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); // Only first run
225+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1);
226+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch);
136227
});
137228

138-
it('does not select if transactionId is falsy', () => {
229+
it('does not select if transactionId is falsy', async () => {
139230
const state = getMockConfirmStateForTransaction(
140231
genUnapprovedContractInteractionConfirmation({
141232
gasFeeTokens: [GAS_FEE_TOKEN_MOCK],
@@ -145,15 +236,23 @@ describe('useAutomaticGasFeeTokenSelect', () => {
145236
// Remove transactionId
146237
state.metamask.transactions = [];
147238
renderHookWithConfirmContextProvider(useAutomaticGasFeeTokenSelect, state);
239+
240+
await flushAsyncUpdates();
241+
148242
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
243+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
149244
});
150245

151-
it('does not select if gasFeeTokens is falsy', () => {
246+
it('does not select if gasFeeTokens is falsy', async () => {
152247
runHook({ gasFeeTokens: [] });
248+
249+
await flushAsyncUpdates();
250+
153251
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
252+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
154253
});
155254

156-
it('does not select first gas fee token if 7702 and future native token', () => {
255+
it('does not select first gas fee token if 7702 and future native token', async () => {
157256
useIsGaslessSupportedMock.mockReturnValue({
158257
isSupported: true,
159258
isSmartTransaction: false,
@@ -168,16 +267,19 @@ describe('useAutomaticGasFeeTokenSelect', () => {
168267
],
169268
});
170269

270+
await flushAsyncUpdates();
271+
171272
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0);
273+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0);
172274
});
173275

174-
it('selects second gas fee token if 7702 and future native token', () => {
276+
it('selects second gas fee token if 7702 and future native token', async () => {
175277
useIsGaslessSupportedMock.mockReturnValue({
176278
isSupported: true,
177279
isSmartTransaction: false,
178280
});
179281

180-
runHook({
282+
const { store } = runHook({
181283
gasFeeTokens: [
182284
{
183285
...GAS_FEE_TOKEN_MOCK,
@@ -187,6 +289,18 @@ describe('useAutomaticGasFeeTokenSelect', () => {
187289
],
188290
});
189291

292+
if (!store) {
293+
throw new Error('Expected store to be defined');
294+
}
295+
296+
await flushAsyncUpdates();
297+
190298
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1);
299+
expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith(
300+
expect.any(String),
301+
GAS_FEE_TOKEN_MOCK.tokenAddress,
302+
);
303+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1);
304+
expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch);
191305
});
192306
});

ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,9 @@ export function useAutomaticGasFeeTokenSelect() {
5151
return;
5252
}
5353

54-
setFirstCheck(false);
55-
5654
if (shouldSelect) {
5755
await selectFirstToken();
56+
setFirstCheck(false);
5857
}
5958
}, [shouldSelect, selectFirstToken, firstCheck, gasFeeTokens, transactionId]);
6059
}

0 commit comments

Comments
 (0)