Skip to content

Commit 2fb33a6

Browse files
runway-github[bot]jpuri
runway-github[bot]
and
jpuri
authored
release(runway): cherry-pick fix: Send functionality related fixes (#36912)
- fix: cp-13.5.0 Send functionality related fixes (#36831) <!-- 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** Multiple small new send implementation related fixes. ## **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: #36787 Fixes: #36789 Fixes: #36757 ## **Manual testing steps** 1. Trigger send flow 2. Check the new amount recipient page ## **Screenshots/Recordings** TODO ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Updates send flow: show fiat currency code when in fiat mode, dedupe/prioritize account recipients, handle missing gas estimates in max amount, and use a new name-resolution error; tests and i18n adjusted. > > - **Send UI/UX**: > - Amount input now displays fiat currency code in fiat mode (`amount.tsx`) using `fiatCurrencyName` from `useCurrencyConversions`. > - **Hooks**: > - `useCurrencyConversions`: exposes `fiatCurrencyName` alongside `fiatCurrencySymbol`. > - `useMaxAmount`: `getEstimatedTotalGas` tolerates missing gas estimates (defaults to 0). > - `useRecipients`: returns account recipients first and de-duplicates contacts by `address`. > - `useNameValidation`: returns `error: 'nameResolutionFailedError'` on failed resolution. > - **Localization**: > - Added `nameResolutionFailedError` string to `app/_locales/en/messages.json` and `en_GB/messages.json`. > - **Tests**: > - Adjusted amount tests to use `fiatCurrencySymbol: '$'` and validate currency code display (`USD`). > - Added max-amount test for missing gas estimates. > - Updated recipient tests for ordering and de-duplication. > - Updated recipient validation test to expect `nameResolutionFailedError`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd214a8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [aed49b1](aed49b1) Co-authored-by: Jyoti Puri <[email protected]>
1 parent e44ec99 commit 2fb33a6

File tree

11 files changed

+120
-20
lines changed

11 files changed

+120
-20
lines changed

‎app/_locales/en/messages.json‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎app/_locales/en_GB/messages.json‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ui/pages/confirmations/components/send/amount/amount.test.tsx‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ describe('Amount', () => {
6262
} as unknown as SendContext.SendContextType);
6363
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
6464
conversionSupportedForAsset: true,
65-
fiatCurrencySymbol: 'USD',
65+
fiatCurrencySymbol: '$',
66+
fiatCurrencyName: 'usd',
6667
getFiatValue: () => '20',
6768
getFiatDisplayValue: () => '$ 20.00',
6869
getNativeValue: () => '20',
@@ -86,7 +87,8 @@ describe('Amount', () => {
8687
} as unknown as SendContext.SendContextType);
8788
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
8889
conversionSupportedForAsset: true,
89-
fiatCurrencySymbol: 'USD',
90+
fiatCurrencySymbol: '$',
91+
fiatCurrencyName: 'usd',
9092
getFiatValue: () => '20',
9193
getFiatDisplayValue: () => '$ 20.00',
9294
getNativeValue: () => '20',
@@ -110,7 +112,8 @@ describe('Amount', () => {
110112
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
111113
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
112114
conversionSupportedForAsset: true,
113-
fiatCurrencySymbol: 'USD',
115+
fiatCurrencySymbol: '$',
116+
fiatCurrencyName: 'usd',
114117
getFiatValue: () => '20',
115118
getFiatDisplayValue: () => '$ 20.00',
116119
getNativeValue: () => '20',
@@ -121,6 +124,7 @@ describe('Amount', () => {
121124
expect(getByText('$ 20.00')).toBeInTheDocument();
122125
fireEvent.click(getByTestId('toggle-fiat-mode'));
123126
expect(getByRole('textbox')).toHaveValue('20');
127+
expect(getByText('USD')).toBeInTheDocument();
124128
fireEvent.change(getByRole('textbox'), { target: { value: 100 } });
125129
expect(getByText('0 NEU')).toBeInTheDocument();
126130
});
@@ -136,7 +140,8 @@ describe('Amount', () => {
136140
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
137141
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
138142
conversionSupportedForAsset: true,
139-
fiatCurrencySymbol: 'USD',
143+
fiatCurrencySymbol: '$',
144+
fiatCurrencyName: 'usd',
140145
getFiatValue: () => '20',
141146
getFiatDisplayValue: () => '$ 20.00',
142147
getNativeValue: () => '20',
@@ -172,7 +177,8 @@ describe('Amount', () => {
172177
} as unknown as SendContext.SendContextType);
173178
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
174179
conversionSupportedForAsset: true,
175-
fiatCurrencySymbol: 'USD',
180+
fiatCurrencySymbol: '$',
181+
fiatCurrencyName: 'usd',
176182
getFiatValue: () => '20',
177183
getFiatDisplayValue: () => '$ 20.00',
178184
getNativeValue: () => '20',
@@ -191,7 +197,8 @@ describe('Amount', () => {
191197
} as unknown as SendContext.SendContextType);
192198
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
193199
conversionSupportedForAsset: true,
194-
fiatCurrencySymbol: 'USD',
200+
fiatCurrencySymbol: '$',
201+
fiatCurrencyName: 'usd',
195202
getFiatValue: () => '20',
196203
getFiatDisplayValue: () => '$ 20.00',
197204
getNativeValue: () => '20',
@@ -252,7 +259,8 @@ describe('Amount', () => {
252259
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
253260
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
254261
conversionSupportedForAsset: true,
255-
fiatCurrencySymbol: 'USD',
262+
fiatCurrencySymbol: '$',
263+
fiatCurrencyName: 'usd',
256264
getFiatValue: () => '20',
257265
getFiatDisplayValue: () => '$ 20.00',
258266
getNativeValue: () => '20',
@@ -272,7 +280,8 @@ describe('Amount', () => {
272280
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
273281
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
274282
conversionSupportedForAsset: false,
275-
fiatCurrencySymbol: 'USD',
283+
fiatCurrencySymbol: '$',
284+
fiatCurrencyName: 'usd',
276285
getFiatValue: () => '20',
277286
getFiatDisplayValue: () => '$ 20.00',
278287
getNativeValue: () => '20',
@@ -292,7 +301,8 @@ describe('Amount', () => {
292301
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
293302
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
294303
conversionSupportedForAsset: false,
295-
fiatCurrencySymbol: 'USD',
304+
fiatCurrencySymbol: '$',
305+
fiatCurrencyName: 'usd',
296306
getFiatValue: () => '20',
297307
getFiatDisplayValue: () => '$ 20.00',
298308
getNativeValue: () => '20',
@@ -315,7 +325,8 @@ describe('Amount', () => {
315325
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
316326
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
317327
conversionSupportedForAsset: true,
318-
fiatCurrencySymbol: 'USD',
328+
fiatCurrencySymbol: '$',
329+
fiatCurrencyName: 'usd',
319330
getFiatValue: () => '20',
320331
getFiatDisplayValue: () => '$ 20.00',
321332
getNativeValue: () => '20',
@@ -336,7 +347,8 @@ describe('Amount', () => {
336347
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
337348
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
338349
conversionSupportedForAsset: true,
339-
fiatCurrencySymbol: 'USD',
350+
fiatCurrencySymbol: '$',
351+
fiatCurrencyName: 'usd',
340352
getFiatValue: () => '20',
341353
getFiatDisplayValue: () => '$ 20.00',
342354
getNativeValue: () => '20',

‎ui/pages/confirmations/components/send/amount/amount.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export const Amount = ({
4545
const [fiatMode, setFiatMode] = useState(false);
4646
const {
4747
conversionSupportedForAsset,
48+
fiatCurrencyName,
4849
getFiatValue,
4950
getFiatDisplayValue,
5051
getNativeValue,
@@ -154,7 +155,9 @@ export const Amount = ({
154155
value={amount}
155156
endAccessory={
156157
<Box display={Display.Flex}>
157-
<Text>{asset?.symbol}</Text>
158+
<Text>
159+
{fiatMode ? fiatCurrencyName?.toUpperCase() : asset?.symbol}
160+
</Text>
158161
{conversionSupportedForAsset && (
159162
<ButtonIcon
160163
ariaLabel="toggle fiat mode"

‎ui/pages/confirmations/hooks/send/useCurrencyConversions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ export const useCurrencyConversions = () => {
132132
asset?.standard !== ERC1155 &&
133133
asset?.standard !== ERC721,
134134
fiatCurrencySymbol: getCurrencySymbol(currentCurrency),
135+
fiatCurrencyName: currentCurrency,
135136
getFiatValue,
136137
getFiatDisplayValue,
137138
getNativeValue,

‎ui/pages/confirmations/hooks/send/useMaxAmount.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,32 @@ describe('useMaxAmount', () => {
5858
expect(result.getMaxAmount()).toEqual('999.999570668411440000');
5959
});
6060

61+
it('not throw error if gas fee estimates is not available', () => {
62+
jest.spyOn(SendContext, 'useSendContext').mockReturnValue({
63+
asset: EVM_NATIVE_ASSET,
64+
chainId: '0x5',
65+
from: MOCK_ADDRESS_1,
66+
} as unknown as SendContext.SendContextType);
67+
useBalanceMock.mockReturnValue({
68+
balance: '10.00',
69+
decimals: 18,
70+
rawBalanceNumeric: new Numeric('1000000000000000000000', 10),
71+
});
72+
const result = renderHook({
73+
...mockState,
74+
metamask: {
75+
...mockState.metamask,
76+
gasFeeEstimatesByChainId: {
77+
'0x5': {
78+
gasFeeEstimates: {},
79+
},
80+
},
81+
},
82+
});
83+
84+
expect(result.getMaxAmount()).toEqual('1000');
85+
});
86+
6187
it('return 0 if balance of native asset is less than gas needed', () => {
6288
jest.spyOn(SendContext, 'useSendContext').mockReturnValue({
6389
asset: EVM_NATIVE_ASSET,

‎ui/pages/confirmations/hooks/send/useMaxAmount.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@ export const getEstimatedTotalGas = (
2828
if (!gasFeeEstimates) {
2929
return new Numeric('0', 10);
3030
}
31-
const {
32-
medium: { suggestedMaxFeePerGas },
33-
} = gasFeeEstimates;
31+
const { medium: { suggestedMaxFeePerGas } = { suggestedMaxFeePerGas: 0 } } =
32+
gasFeeEstimates;
3433
const totalGas = new Numeric(
3534
suggestedMaxFeePerGas * NATIVE_TRANSFER_GAS_LIMIT,
3635
10,

‎ui/pages/confirmations/hooks/send/useNameValidation.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export const useNameValidation = () => {
1818

1919
if (resolutions.length === 0) {
2020
return {
21-
error: 'ensUnknownError',
21+
error: 'nameResolutionFailedError',
2222
};
2323
}
2424

@@ -33,7 +33,7 @@ export const useNameValidation = () => {
3333
}
3434

3535
return {
36-
error: 'ensUnknownError',
36+
error: 'nameResolutionFailedError',
3737
};
3838
},
3939
[fetchResolutions],

‎ui/pages/confirmations/hooks/send/useRecipientValidation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ describe('useRecipientValidation', () => {
206206
jest.spyOn(NameValidation, 'useNameValidation').mockReturnValue({
207207
validateName: () =>
208208
Promise.resolve({
209-
error: 'ensUnknownError',
209+
error: 'nameResolutionFailedError',
210210
}),
211211
});
212212

‎ui/pages/confirmations/hooks/send/useRecipients.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,49 @@ describe('useRecipients', () => {
4646
const { result } = renderHookWithProvider(() => useRecipients(), mockState);
4747

4848
expect(result.current).toEqual([
49+
...mockAccountRecipients,
4950
...mockContactRecipients,
51+
]);
52+
});
53+
54+
it('it returns unique recipients', () => {
55+
const mockAccountRecipients: Recipient[] = [
56+
{
57+
address: '0x5678901234',
58+
walletName: 'Wallet 1',
59+
accountGroupName: 'Account 1',
60+
},
61+
{
62+
address: '0xfedcba5678',
63+
walletName: 'Wallet 2',
64+
accountGroupName: 'Account 2',
65+
},
66+
];
67+
68+
const mockContactRecipients: Recipient[] = [
69+
{ address: '0x1234567890', contactName: 'Contact 1' },
70+
{ address: '0xabcdef1234', contactName: 'Contact 2' },
71+
{
72+
address: '0x1234567890',
73+
walletName: 'Wallet 11',
74+
accountGroupName: 'Account 11',
75+
},
76+
{
77+
address: '0x5678901234',
78+
walletName: 'Wallet 12',
79+
accountGroupName: 'Account 12',
80+
},
81+
];
82+
83+
mockUseContactRecipients.mockReturnValue(mockContactRecipients);
84+
mockUseAccountRecipients.mockReturnValue(mockAccountRecipients);
85+
86+
const { result } = renderHookWithProvider(() => useRecipients(), mockState);
87+
88+
expect(result.current).toEqual([
5089
...mockAccountRecipients,
90+
mockContactRecipients[0],
91+
mockContactRecipients[1],
5192
]);
5293
});
5394

0 commit comments

Comments
 (0)