From 7e88180085c44740eb477998bfb9da124ca9f90a Mon Sep 17 00:00:00 2001 From: MetaMask Bot Date: Thu, 26 Sep 2024 18:50:19 +0000 Subject: [PATCH 01/14] Version v12.3.1 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 199b83700dde..c3d631ae1863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [12.3.1] + ## [12.3.0] ### Added - Added the ability to name accounts during the snap account creation flow ([#25191](https://github.com/MetaMask/metamask-extension/pull/25191)) @@ -5106,7 +5108,8 @@ Update styles and spacing on the critical error page ([#20350](https://github.c - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.3.0...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.3.1...HEAD +[12.3.1]: https://github.com/MetaMask/metamask-extension/compare/v12.3.0...v12.3.1 [12.3.0]: https://github.com/MetaMask/metamask-extension/compare/v12.2.4...v12.3.0 [12.2.4]: https://github.com/MetaMask/metamask-extension/compare/v12.2.3...v12.2.4 [12.2.3]: https://github.com/MetaMask/metamask-extension/compare/v12.2.2...v12.2.3 diff --git a/package.json b/package.json index 939c5ce3ff10..51894461c76e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "12.3.0", + "version": "12.3.1", "private": true, "repository": { "type": "git", From 5f93766df41b787d2f0d391a8ba0fa79c6bda52b Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Thu, 26 Sep 2024 17:36:28 -0230 Subject: [PATCH 02/14] fix: fix changes that were introduced in (#26807) but broken by 3e1094af (#27435) This PR correctly applies the changes from #26807 and #26924 to master. These were broken, probably by careless human error, in 3e1094af --------- Co-authored-by: Matteo Scurati --- app/scripts/metamask-controller.js | 2 -- test/e2e/tests/metrics/nft-detection-metrics.spec.js | 1 - test/e2e/tests/metrics/token-detection-metrics.spec.js | 1 - test/e2e/tests/metrics/wallet-created.spec.js | 1 - ui/components/multichain/global-menu/global-menu.js | 1 + .../notification-detail-button/notification-detail-button.tsx | 2 +- .../creation-successful/creation-successful.js | 4 ---- 7 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 141fab5aebc8..ebffc180f95f 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1488,8 +1488,6 @@ export default class MetamaskController extends EventEmitter { notification_id: notification.id, notification_type: notification.type, chain_id: notification?.chain_id, - notification_is_read: notification.isRead, - click_type: 'push_notification', }, }); }, diff --git a/test/e2e/tests/metrics/nft-detection-metrics.spec.js b/test/e2e/tests/metrics/nft-detection-metrics.spec.js index 5b635a725e20..3c77fdb66731 100644 --- a/test/e2e/tests/metrics/nft-detection-metrics.spec.js +++ b/test/e2e/tests/metrics/nft-detection-metrics.spec.js @@ -102,7 +102,6 @@ describe('Nft detection event @no-mmi', function () { chain_id: '0x539', environment_type: 'fullscreen', is_profile_syncing_enabled: null, - is_signed_in: false, }); assert.deepStrictEqual(events[2].properties, { nft_autodetection_enabled: true, diff --git a/test/e2e/tests/metrics/token-detection-metrics.spec.js b/test/e2e/tests/metrics/token-detection-metrics.spec.js index 5115343f5a39..669aff0a9290 100644 --- a/test/e2e/tests/metrics/token-detection-metrics.spec.js +++ b/test/e2e/tests/metrics/token-detection-metrics.spec.js @@ -99,7 +99,6 @@ describe('Token detection event @no-mmi', function () { chain_id: '0x539', environment_type: 'fullscreen', is_profile_syncing_enabled: null, - is_signed_in: false, }); assert.deepStrictEqual(events[2].properties, { token_detection_enabled: true, diff --git a/test/e2e/tests/metrics/wallet-created.spec.js b/test/e2e/tests/metrics/wallet-created.spec.js index bc07cfdeea75..890ac9342a8a 100644 --- a/test/e2e/tests/metrics/wallet-created.spec.js +++ b/test/e2e/tests/metrics/wallet-created.spec.js @@ -88,7 +88,6 @@ describe('Wallet Created Events @no-mmi', function () { chain_id: '0x539', environment_type: 'fullscreen', is_profile_syncing_enabled: null, - is_signed_in: false, }); }, ); diff --git a/ui/components/multichain/global-menu/global-menu.js b/ui/components/multichain/global-menu/global-menu.js index 09fdce45f974..ab33de326ee7 100644 --- a/ui/components/multichain/global-menu/global-menu.js +++ b/ui/components/multichain/global-menu/global-menu.js @@ -148,6 +148,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement, isOpen }) => { if (shouldShowEnableModal) { trackEvent({ + category: MetaMetricsEventCategory.NotificationsActivationFlow, event: MetaMetricsEventName.NotificationsActivated, properties: { action_type: 'started', diff --git a/ui/components/multichain/notification-detail-button/notification-detail-button.tsx b/ui/components/multichain/notification-detail-button/notification-detail-button.tsx index 82a79c4fcf91..b4c243ef9b7d 100644 --- a/ui/components/multichain/notification-detail-button/notification-detail-button.tsx +++ b/ui/components/multichain/notification-detail-button/notification-detail-button.tsx @@ -38,7 +38,7 @@ export const NotificationDetailButton = ({ const onClick = () => { trackEvent({ category: MetaMetricsEventCategory.NotificationInteraction, - event: MetaMetricsEventName.NotificationClicked, + event: MetaMetricsEventName.NotificationDetailClicked, properties: { notification_id: notification.id, notification_type: notification.type, diff --git a/ui/pages/onboarding-flow/creation-successful/creation-successful.js b/ui/pages/onboarding-flow/creation-successful/creation-successful.js index 79321c7c5991..fab463e5b685 100644 --- a/ui/pages/onboarding-flow/creation-successful/creation-successful.js +++ b/ui/pages/onboarding-flow/creation-successful/creation-successful.js @@ -25,7 +25,6 @@ import { import { MetaMetricsContext } from '../../../contexts/metametrics'; import { useCreateSession } from '../../../hooks/metamask-notifications/useCreateSession'; import { selectIsProfileSyncingEnabled } from '../../../selectors/metamask-notifications/profile-syncing'; -import { selectIsSignedIn } from '../../../selectors/metamask-notifications/authentication'; export default function CreationSuccessful() { const history = useHistory(); @@ -37,8 +36,6 @@ export default function CreationSuccessful() { const isProfileSyncingEnabled = useSelector(selectIsProfileSyncingEnabled); - const isSignedIn = useSelector(selectIsSignedIn); - return (
@@ -118,7 +115,6 @@ export default function CreationSuccessful() { event: MetaMetricsEventName.OnboardingWalletCreationComplete, properties: { method: firstTimeFlowType, - is_signed_in: isSignedIn, is_profile_syncing_enabled: isProfileSyncingEnabled, }, }); From 190572fbe6943520e5e7bcae498ab7080e4d3146 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Fri, 27 Sep 2024 12:41:01 -0700 Subject: [PATCH 03/14] fix: duplicate network validation in 12.3 (#27463) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes issue [27456](https://github.com/MetaMask/metamask-extension/issues/27456) in 12.3. If you try to add a new network with the same RPC URL and chain id as an existing network, the link presented in the error message does not work and throws an error: https://github.com/user-attachments/assets/ebcade54-e49e-4a04-9ed6-396f2458f77a This link in the error message was intended for the network redesign and should stay behind the feature flag. Duplicate chain IDs are allowed in this version of network managment. So this PR makes it back into a warning like it would have been in 12.2: Screenshot 2024-09-27 at 12 02 49 PM There's nothing to fix in develop since it is already on the new network controller / UI [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27463?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27456 ## **Manual testing steps** 1. Try to add a new network with the same RPC URL and chain id as an existing network. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/develop/.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/develop/.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. --- ui/pages/settings/networks-tab/networks-form/networks-form.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/pages/settings/networks-tab/networks-form/networks-form.js b/ui/pages/settings/networks-tab/networks-form/networks-form.js index 4998e7df1a43..247936a76621 100644 --- a/ui/pages/settings/networks-tab/networks-form/networks-form.js +++ b/ui/pages/settings/networks-tab/networks-form/networks-form.js @@ -508,6 +508,7 @@ const NetworksForm = ({ } if ( + networkMenuRedesign && addNewNetwork && networksList.some( (network) => From 31d1f143b84f45b7bd3cd9ef22cc8300a277839a Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 27 Sep 2024 17:14:37 -0230 Subject: [PATCH 04/14] fix: Handle null return value from getMethodData to prevent destructuring error (#27457) (#27462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The original code assumes that getMethodData will always return an object with a name property. However, in certain instances, getMethodData can return null. When this happens, destructuring the name property from null causes a runtime error. To address this issue, the code has been updated to use optional chaining. This ensures that if getMethodData returns null, the destructuring will not occur, and contractMethodName will be set to undefined instead of causing an error. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27457?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27436 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/develop/.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/develop/.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. Co-authored-by: Pedro Figueiredo --- app/scripts/lib/transaction/metrics.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/transaction/metrics.ts b/app/scripts/lib/transaction/metrics.ts index 6ce19ec65303..39bf23b7e74c 100644 --- a/app/scripts/lib/transaction/metrics.ts +++ b/app/scripts/lib/transaction/metrics.ts @@ -810,10 +810,10 @@ async function buildEventFragmentProperties({ let contractMethodName; if (transactionMeta.txParams.data) { - const { name } = await transactionMetricsRequest.getMethodData( + const methodData = await transactionMetricsRequest.getMethodData( transactionMeta.txParams.data, ); - contractMethodName = name; + contractMethodName = methodData?.name; } // TODO: Replace `any` with type From 98a6968932d267b7b837fea80f8f0a90fb02e0e4 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 30 Sep 2024 10:44:35 -0230 Subject: [PATCH 05/14] chore: Update changelog for v12.3.1 (#27465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Update changelog for v12.3.1 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27465?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **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/develop/.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/develop/.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. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3d631ae1863..d4825ab021bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [12.3.1] +### Fixed +- Fix duplicate network validation ([#27463](https://github.com/MetaMask/metamask-extension/pull/27463)) +- Fix notification metrics ([#26807](https://github.com/MetaMask/metamask-extension/pull/26807)) +- Fix transaction metrics ([#27457](https://github.com/MetaMask/metamask-extension/pull/27457)) ## [12.3.0] ### Added From 2ede67e2296f42224fab2c97784dfe83f2c47329 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 30 Sep 2024 16:56:16 -0230 Subject: [PATCH 06/14] chore: Fix link in change entry (#27508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The "Fix notification metrics" change entry linked a PR where some of these metrics were originally introduced, but it should have linked the PR where they were fixed. The link has been updated. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27508?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **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/develop/.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/develop/.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. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4825ab021bc..9dd79055e0c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [12.3.1] ### Fixed - Fix duplicate network validation ([#27463](https://github.com/MetaMask/metamask-extension/pull/27463)) -- Fix notification metrics ([#26807](https://github.com/MetaMask/metamask-extension/pull/26807)) +- Fix notification metrics ([#27435](https://github.com/MetaMask/metamask-extension/pull/27435)) - Fix transaction metrics ([#27457](https://github.com/MetaMask/metamask-extension/pull/27457)) ## [12.3.0] From 80b85f4cd95cc06c5aa7d1094b692a1105b31c33 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Tue, 1 Oct 2024 09:45:30 +0100 Subject: [PATCH 07/14] feat: Custom header for wallet initiated confirmations (#27391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The back button brings the user to the stepper for ERC20 token transfer. This PR includes creating a placeholder component for token transfer confirmations. It also includes unit tests. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27391?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3218 ## **Manual testing steps** 1. Enable redesign on developer options. 2. Initiate transfer from an erc20 token in the wallet. 3. The confirmation in the screenshot below should appear. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-09-25 at 11 14 03 ## **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/develop/.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/develop/.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. --- app/_locales/en/messages.json | 3 + .../confirmations/contract-interaction.ts | 40 ----- test/data/confirmations/helper.ts | 24 ++- .../confirmations/set-approval-for-all.ts | 27 +++ test/data/confirmations/token-approve.ts | 27 +++ test/data/confirmations/token-transfer.ts | 32 ++++ .../header/__snapshots__/header.test.tsx.snap | 164 ++++++++++++++++++ .../wallet-initiated-header.test.tsx.snap | 40 +++++ .../components/confirm/header/header.test.tsx | 21 +++ .../components/confirm/header/header.tsx | 18 ++ .../header/wallet-initiated-header.test.tsx | 21 +++ .../header/wallet-initiated-header.tsx | 103 +++++++++++ .../approve/hooks/use-received-token.test.ts | 6 +- .../components/confirm/info/info.tsx | 2 + .../token-transfer.test.tsx.snap | 3 + .../token-transfer/token-transfer.stories.tsx | 26 +++ .../token-transfer/token-transfer.test.tsx | 26 +++ .../info/token-transfer/token-transfer.tsx | 5 + .../components/confirm/nav/nav.tsx | 2 +- .../title/hooks/useCurrentSpendingCap.test.ts | 6 +- ui/pages/confirmations/utils/confirm.ts | 1 + 21 files changed, 542 insertions(+), 55 deletions(-) create mode 100644 test/data/confirmations/set-approval-for-all.ts create mode 100644 test/data/confirmations/token-approve.ts create mode 100644 test/data/confirmations/token-transfer.ts create mode 100644 ui/pages/confirmations/components/confirm/header/__snapshots__/wallet-initiated-header.test.tsx.snap create mode 100644 ui/pages/confirmations/components/confirm/header/wallet-initiated-header.test.tsx create mode 100644 ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index b42895983048..330573566f70 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -4544,6 +4544,9 @@ "revealTheSeedPhrase": { "message": "Reveal seed phrase" }, + "review": { + "message": "Review" + }, "reviewAlert": { "message": "Review alert" }, diff --git a/test/data/confirmations/contract-interaction.ts b/test/data/confirmations/contract-interaction.ts index 0556789ccdb2..507a27a48dc3 100644 --- a/test/data/confirmations/contract-interaction.ts +++ b/test/data/confirmations/contract-interaction.ts @@ -161,43 +161,3 @@ export const genUnapprovedContractInteractionConfirmation = ({ userFeeLevel: 'medium', verifiedOnBlockchain: false, } as SignatureRequestType); - -export const genUnapprovedApproveConfirmation = ({ - address = CONTRACT_INTERACTION_SENDER_ADDRESS, - chainId = CHAIN_ID, -}: { - address?: Hex; - chainId?: string; -} = {}) => ({ - ...genUnapprovedContractInteractionConfirmation({ chainId }), - txParams: { - from: address, - data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', - gas: '0x16a92', - to: '0x076146c765189d51be3160a2140cf80bfc73ad68', - value: '0x0', - maxFeePerGas: '0x5b06b0c0d', - maxPriorityFeePerGas: '0x59682f00', - }, - type: TransactionType.tokenMethodApprove, -}); - -export const genUnapprovedSetApprovalForAllConfirmation = ({ - address = CONTRACT_INTERACTION_SENDER_ADDRESS, - chainId = CHAIN_ID, -}: { - address?: Hex; - chainId?: string; -} = {}) => ({ - ...genUnapprovedContractInteractionConfirmation({ chainId }), - txParams: { - from: address, - data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', - gas: '0x16a92', - to: '0x076146c765189d51be3160a2140cf80bfc73ad68', - value: '0x0', - maxFeePerGas: '0x5b06b0c0d', - maxPriorityFeePerGas: '0x59682f00', - }, - type: TransactionType.tokenMethodSetApprovalForAll, -}); diff --git a/test/data/confirmations/helper.ts b/test/data/confirmations/helper.ts index 9eb8bb234768..6669c043d0ea 100644 --- a/test/data/confirmations/helper.ts +++ b/test/data/confirmations/helper.ts @@ -1,18 +1,17 @@ import { ApprovalType } from '@metamask/controller-utils'; import { merge } from 'lodash'; +import { CHAIN_IDS } from '../../../shared/constants/network'; import { Confirmation, SignatureRequestType, } from '../../../ui/pages/confirmations/types/confirm'; import mockState from '../mock-state.json'; -import { CHAIN_IDS } from '../../../shared/constants/network'; -import { - genUnapprovedApproveConfirmation, - genUnapprovedContractInteractionConfirmation, - genUnapprovedSetApprovalForAllConfirmation, -} from './contract-interaction'; +import { genUnapprovedContractInteractionConfirmation } from './contract-interaction'; import { unapprovedPersonalSignMsg } from './personal_sign'; +import { genUnapprovedSetApprovalForAllConfirmation } from './set-approval-for-all'; +import { genUnapprovedApproveConfirmation } from './token-approve'; +import { genUnapprovedTokenTransferConfirmation } from './token-transfer'; import { unapprovedTypedSignMsgV4 } from './typed_sign'; type RootState = { metamask: Record } & Record< @@ -183,3 +182,16 @@ export const getMockSetApprovalForAllConfirmState = () => { genUnapprovedSetApprovalForAllConfirmation({ chainId: '0x5' }), ); }; + +export const getMockTokenTransferConfirmState = ({ + isWalletInitiatedConfirmation = false, +}: { + isWalletInitiatedConfirmation?: boolean; +}) => { + return getMockConfirmStateForTransaction( + genUnapprovedTokenTransferConfirmation({ + chainId: '0x5', + isWalletInitiatedConfirmation, + }), + ); +}; diff --git a/test/data/confirmations/set-approval-for-all.ts b/test/data/confirmations/set-approval-for-all.ts new file mode 100644 index 000000000000..ca997f6212af --- /dev/null +++ b/test/data/confirmations/set-approval-for-all.ts @@ -0,0 +1,27 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedSetApprovalForAllConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, +}: { + address?: Hex; + chainId?: string; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodSetApprovalForAll, +}); diff --git a/test/data/confirmations/token-approve.ts b/test/data/confirmations/token-approve.ts new file mode 100644 index 000000000000..c77d59101a99 --- /dev/null +++ b/test/data/confirmations/token-approve.ts @@ -0,0 +1,27 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedApproveConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, +}: { + address?: Hex; + chainId?: string; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodApprove, +}); diff --git a/test/data/confirmations/token-transfer.ts b/test/data/confirmations/token-transfer.ts new file mode 100644 index 000000000000..22d0cb2d00b4 --- /dev/null +++ b/test/data/confirmations/token-transfer.ts @@ -0,0 +1,32 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedTokenTransferConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, + isWalletInitiatedConfirmation = false, +}: { + address?: Hex; + chainId?: string; + isWalletInitiatedConfirmation?: boolean; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodTransfer, + origin: isWalletInitiatedConfirmation + ? 'metamask' + : 'https://metamask.github.io', +}); diff --git a/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap b/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap index 46bf53c2a7bc..1af0810d285f 100644 --- a/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap @@ -113,6 +113,170 @@ exports[`Header should match snapshot with signature confirmation 1`] = `
`; +exports[`Header should match snapshot with token transfer confirmation initiated in a dApp 1`] = ` +
+
+
+
+
+
+
+ + + + + +
+
+
+
+ G +
+
+
+

+

+ Goerli +

+
+
+
+
+
+
+ +
+
+
+ +
+
+
+
+
+`; + +exports[`Header should match snapshot with token transfer confirmation initiated in the wallet 1`] = ` +
+
+ +

+ Review +

+
+ +
+
+
+`; + exports[`Header should match snapshot with transaction confirmation 1`] = `
should match snapshot 1`] = ` +
+
+ +

+ Review +

+
+ +
+
+
+`; diff --git a/ui/pages/confirmations/components/confirm/header/header.test.tsx b/ui/pages/confirmations/components/confirm/header/header.test.tsx index c6b8481c01fc..841c0196ae29 100644 --- a/ui/pages/confirmations/components/confirm/header/header.test.tsx +++ b/ui/pages/confirmations/components/confirm/header/header.test.tsx @@ -4,6 +4,7 @@ import { DefaultRootState } from 'react-redux'; import { getMockContractInteractionConfirmState, + getMockTokenTransferConfirmState, getMockTypedSignConfirmState, } from '../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; @@ -28,6 +29,26 @@ describe('Header', () => { expect(container).toMatchSnapshot(); }); + it('should match snapshot with token transfer confirmation initiated in a dApp', () => { + const { container } = render( + getMockTokenTransferConfirmState({ + isWalletInitiatedConfirmation: false, + }), + ); + + expect(container).toMatchSnapshot(); + }); + + it('should match snapshot with token transfer confirmation initiated in the wallet', () => { + const { container } = render( + getMockTokenTransferConfirmState({ + isWalletInitiatedConfirmation: true, + }), + ); + + expect(container).toMatchSnapshot(); + }); + it('contains network name and account name', () => { const { getByText } = render(); expect(getByText('Test Account')).toBeInTheDocument(); diff --git a/ui/pages/confirmations/components/confirm/header/header.tsx b/ui/pages/confirmations/components/confirm/header/header.tsx index 255384c58b82..9c113effe6a5 100644 --- a/ui/pages/confirmations/components/confirm/header/header.tsx +++ b/ui/pages/confirmations/components/confirm/header/header.tsx @@ -1,3 +1,7 @@ +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; import React from 'react'; import { AvatarNetwork, @@ -14,15 +18,29 @@ import { TextVariant, } from '../../../../../helpers/constants/design-system'; import { getAvatarNetworkColor } from '../../../../../helpers/utils/accounts'; +import { useConfirmContext } from '../../../context/confirm'; import useConfirmationNetworkInfo from '../../../hooks/useConfirmationNetworkInfo'; import useConfirmationRecipientInfo from '../../../hooks/useConfirmationRecipientInfo'; +import { Confirmation } from '../../../types/confirm'; import HeaderInfo from './header-info'; +import { WalletInitiatedHeader } from './wallet-initiated-header'; const Header = () => { const { networkImageUrl, networkDisplayName } = useConfirmationNetworkInfo(); const { senderAddress: fromAddress, senderName: fromName } = useConfirmationRecipientInfo(); + const { currentConfirmation } = useConfirmContext(); + + if (currentConfirmation?.type === TransactionType.tokenMethodTransfer) { + const isWalletInitiated = + (currentConfirmation as TransactionMeta).origin === 'metamask'; + + if (isWalletInitiated) { + return ; + } + } + return ( { + const store = configureStore(state); + return renderWithConfirmContextProvider(, store); +}; + +describe('', () => { + it.only('should match snapshot', () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx b/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx new file mode 100644 index 000000000000..c1bca06c74b0 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx @@ -0,0 +1,103 @@ +import { TransactionMeta } from '@metamask/transaction-controller'; +import React, { useCallback } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { useHistory } from 'react-router-dom'; +import { AssetType } from '../../../../../../shared/constants/transaction'; +import { + Box, + ButtonIcon, + ButtonIconSize, + IconName, + Text, +} from '../../../../../components/component-library'; +import { clearConfirmTransaction } from '../../../../../ducks/confirm-transaction/confirm-transaction.duck'; +import { editExistingTransaction } from '../../../../../ducks/send'; +import { + AlignItems, + BackgroundColor, + BorderRadius, + Display, + FlexDirection, + IconColor, + JustifyContent, + TextColor, + TextVariant, +} from '../../../../../helpers/constants/design-system'; +import { SEND_ROUTE } from '../../../../../helpers/constants/routes'; +import { useI18nContext } from '../../../../../hooks/useI18nContext'; +import { + setConfirmationAdvancedDetailsOpen, + showSendTokenPage, +} from '../../../../../store/actions'; +import { useConfirmContext } from '../../../context/confirm'; +import { selectConfirmationAdvancedDetailsOpen } from '../../../selectors/preferences'; + +export const WalletInitiatedHeader = () => { + const t = useI18nContext(); + const dispatch = useDispatch(); + const history = useHistory(); + + const { currentConfirmation } = useConfirmContext(); + + const showAdvancedDetails = useSelector( + selectConfirmationAdvancedDetailsOpen, + ); + + const setShowAdvancedDetails = (value: boolean): void => { + dispatch(setConfirmationAdvancedDetailsOpen(value)); + }; + + const handleBackButtonClick = useCallback(async () => { + const { id } = currentConfirmation; + + await dispatch(editExistingTransaction(AssetType.token, id.toString())); + dispatch(clearConfirmTransaction()); + dispatch(showSendTokenPage()); + + history.push(SEND_ROUTE); + }, [currentConfirmation, dispatch, history]); + + return ( + + + + {t('review')} + + + { + setShowAdvancedDetails(!showAdvancedDetails); + }} + /> + + + ); +}; diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts index 874e817cc20a..a6e92167e558 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts @@ -1,10 +1,8 @@ import { TransactionMeta } from '@metamask/transaction-controller'; -import { - CONTRACT_INTERACTION_SENDER_ADDRESS, - genUnapprovedApproveConfirmation, -} from '../../../../../../../../test/data/confirmations/contract-interaction'; +import { CONTRACT_INTERACTION_SENDER_ADDRESS } from '../../../../../../../../test/data/confirmations/contract-interaction'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; +import { genUnapprovedApproveConfirmation } from '../../../../../../../../test/data/confirmations/token-approve'; import { renderHookWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import { useAccountTotalFiatBalance } from '../../../../../../../hooks/useAccountTotalFiatBalance'; import { useReceivedToken } from './use-received-token'; diff --git a/ui/pages/confirmations/components/confirm/info/info.tsx b/ui/pages/confirmations/components/confirm/info/info.tsx index 5a9c4757158e..3e87f4f7908c 100644 --- a/ui/pages/confirmations/components/confirm/info/info.tsx +++ b/ui/pages/confirmations/components/confirm/info/info.tsx @@ -6,6 +6,7 @@ import ApproveInfo from './approve/approve'; import BaseTransactionInfo from './base-transaction-info/base-transaction-info'; import PersonalSignInfo from './personal-sign/personal-sign'; import SetApprovalForAllInfo from './set-approval-for-all-info/set-approval-for-all-info'; +import TokenTransferInfo from './token-transfer/token-transfer'; import TypedSignV1Info from './typed-sign-v1/typed-sign-v1'; import TypedSignInfo from './typed-sign/typed-sign'; @@ -29,6 +30,7 @@ const Info = () => { [TransactionType.tokenMethodIncreaseAllowance]: () => ApproveInfo, [TransactionType.tokenMethodSetApprovalForAll]: () => SetApprovalForAllInfo, + [TransactionType.tokenMethodTransfer]: () => TokenTransferInfo, }), [currentConfirmation], ); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap new file mode 100644 index 000000000000..c3aa8e4e26ea --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`TokenTransferInfo renders correctly 1`] = `
`; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx new file mode 100644 index 000000000000..384a8f161e9b --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import { Provider } from 'react-redux'; +import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import configureStore from '../../../../../../store/store'; +import { ConfirmContextProvider } from '../../../../context/confirm'; +import TokenTransferInfo from './token-transfer'; + +const store = configureStore(getMockTokenTransferConfirmState({})); + +const Story = { + title: 'Components/App/Confirm/info/TokenTransferInfo', + component: TokenTransferInfo, + decorators: [ + (story: () => any) => ( + + {story()} + + ), + ], +}; + +export default Story; + +export const DefaultStory = () => ; + +DefaultStory.storyName = 'Default'; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx new file mode 100644 index 000000000000..186505ee7740 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import TokenTransferInfo from './token-transfer'; + +jest.mock( + '../../../../../../components/app/alert-system/contexts/alertMetricsContext', + () => ({ + useAlertMetrics: jest.fn(() => ({ + trackAlertMetrics: jest.fn(), + })), + }), +); + +describe('TokenTransferInfo', () => { + it('renders correctly', () => { + const state = getMockTokenTransferConfirmState({}); + const mockStore = configureMockStore([])(state); + const { container } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx new file mode 100644 index 000000000000..8da9493ebbc4 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx @@ -0,0 +1,5 @@ +const TokenTransferInfo = () => { + return null; +}; + +export default TokenTransferInfo; diff --git a/ui/pages/confirmations/components/confirm/nav/nav.tsx b/ui/pages/confirmations/components/confirm/nav/nav.tsx index 2fd394f18ae2..6546b882b784 100644 --- a/ui/pages/confirmations/components/confirm/nav/nav.tsx +++ b/ui/pages/confirmations/components/confirm/nav/nav.tsx @@ -32,9 +32,9 @@ import { import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { pendingConfirmationsSortedSelector } from '../../../../../selectors'; import { rejectPendingApproval } from '../../../../../store/actions'; +import { useConfirmContext } from '../../../context/confirm'; import { useQueuedConfirmationsEvent } from '../../../hooks/useQueuedConfirmationEvents'; import { isSignatureApprovalRequest } from '../../../utils'; -import { useConfirmContext } from '../../../context/confirm'; const Nav = () => { const history = useHistory(); diff --git a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts index 40886608870b..9bea069d0935 100644 --- a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts +++ b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts @@ -1,8 +1,6 @@ import { TransactionMeta } from '@metamask/transaction-controller'; -import { - CONTRACT_INTERACTION_SENDER_ADDRESS, - genUnapprovedApproveConfirmation, -} from '../../../../../../../test/data/confirmations/contract-interaction'; +import { CONTRACT_INTERACTION_SENDER_ADDRESS } from '../../../../../../../test/data/confirmations/contract-interaction'; +import { genUnapprovedApproveConfirmation } from '../../../../../../../test/data/confirmations/token-approve'; import mockState from '../../../../../../../test/data/mock-state.json'; import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; import { useCurrentSpendingCap } from './useCurrentSpendingCap'; diff --git a/ui/pages/confirmations/utils/confirm.ts b/ui/pages/confirmations/utils/confirm.ts index 8c2846b6b69a..41ffd2832169 100644 --- a/ui/pages/confirmations/utils/confirm.ts +++ b/ui/pages/confirmations/utils/confirm.ts @@ -26,6 +26,7 @@ export const REDESIGN_USER_TRANSACTION_TYPES = [ export const REDESIGN_DEV_TRANSACTION_TYPES = [ ...REDESIGN_USER_TRANSACTION_TYPES, + TransactionType.tokenMethodTransfer, ]; const SIGNATURE_APPROVAL_TYPES = [ From 9b50f596b706585d6075cacd90e1aeba9e4acf20 Mon Sep 17 00:00:00 2001 From: Nidhi Kumari Date: Tue, 1 Oct 2024 09:57:14 +0100 Subject: [PATCH 08/14] fix: updated ui for connect and review page (#27478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is to update the following UI changes in the connect and the new permissions page ## **Description** * Fix the Typography * Update background color on Connect Page * Don't show header on review permissions page ## **Related issues** Fixes: ## **Manual testing steps** 1. Run extension with `CHAIN_PERMISSIONS=1 yarn start` 2. Check the following changes in Connect Page: Background color is fixed, Typography looks aligned with design, Bottom learn more message 3. Fix Permissions Page: Typography is fixed, the list item has border radius and proper padding, three dot menu replaced by Edit button ## **Screenshots/Recordings** ### **Before** ![Screenshot 2024-09-30 at 3 53 23 AM](https://github.com/user-attachments/assets/98aeb9f2-df44-46af-b2f2-777002228c64) ![Screenshot 2024-09-30 at 3 53 09 AM](https://github.com/user-attachments/assets/ade555e9-fbb2-424b-bc15-31af8a50e1b4) ### **After** ## Connect Page ![Screenshot 2024-09-30 at 4 01 38 AM](https://github.com/user-attachments/assets/818b2ef1-bf43-4eb3-890f-8b45eb29c50b) ## Permission Page ![Screenshot 2024-09-30 at 3 52 38 AM](https://github.com/user-attachments/assets/dd488013-2cd8-41df-ba5f-681daf48df93) ## **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/develop/.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/develop/.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. --- app/_locales/en/messages.json | 8 + ...ite-cell-connection-list-item.test.js.snap | 7 +- .../site-cell-connection-list-item.js | 36 +-- .../site-cell-connection-list-item.test.js | 4 + .../site-cell/site-cell.tsx | 85 +++--- .../__snapshots__/connect-page.test.tsx.snap | 276 ++++++++++-------- .../connect-page/connect-page.test.tsx | 2 +- .../connect-page/connect-page.tsx | 64 ++-- ui/pages/permissions-connect/index.scss | 4 + ui/pages/routes/routes.component.js | 11 + 10 files changed, 294 insertions(+), 203 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 330573566f70..b880d92ad468 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1186,9 +1186,17 @@ "message": "Connected with" }, "connectedWithAccount": { + "message": "$1 accounts connected", + "description": "$1 represents account length" + }, + "connectedWithAccountName": { "message": "Connected with $1", "description": "$1 represents account name" }, + "connectedWithNetworks": { + "message": "$1 networks connected", + "description": "$1 represents network length" + }, "connecting": { "message": "Connecting" }, diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap b/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap index 5dc31c8e210a..ae198ab79882 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap @@ -3,7 +3,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] = `

Title

@@ -27,7 +27,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] = class="mm-box mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--align-items-center" > Unconnected Message @@ -38,6 +38,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] =
diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js index 85e50b0b0fed..be2aafb7257b 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js @@ -15,10 +15,7 @@ import { AvatarIcon, AvatarIconSize, Box, - ButtonIcon, - ButtonIconSize, ButtonLink, - IconName, Text, } from '../../../../component-library'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; @@ -31,6 +28,8 @@ export const SiteCellConnectionListItem = ({ isConnectFlow, onClick, content, + paddingTopValue, + paddingBottomValue, }) => { const t = useI18nContext(); @@ -42,9 +41,10 @@ export const SiteCellConnectionListItem = ({ alignItems={AlignItems.baseline} width={BlockSize.Full} backgroundColor={BackgroundColor.backgroundDefault} - padding={4} gap={4} className="multichain-connection-list-item" + paddingTop={paddingTopValue} + paddingBottom={paddingBottomValue} > - + {title} {isConnectFlow ? unconnectedMessage : connectedMessage} @@ -79,17 +79,9 @@ export const SiteCellConnectionListItem = ({ {content} - {isConnectFlow ? ( - onClick()}>{t('edit')} - ) : ( - onClick()} - size={ButtonIconSize.Sm} - /> - )} + onClick()} data-testid="edit"> + {t('edit')} + ); }; @@ -109,6 +101,16 @@ SiteCellConnectionListItem.propTypes = { */ connectedMessage: PropTypes.string, + /** + * Padding Top Value to keep the padding between list items same + */ + paddingTopValue: PropTypes.number, + + /** + * Padding Bottom Value to keep the padding between list items same + */ + paddingBottomValue: PropTypes.number, + /** * The message that should be displayed when there are no connected accounts */ diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js index 613f07f348f3..954d1e2d1fc2 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js @@ -36,4 +36,8 @@ describe('SiteCellConnectionListItem', () => { it('returns wallet icon correctly', () => { expect(getByText('Title')).toBeDefined(); }); + + it('returns edit button correctly', () => { + expect(getByTestId('edit')).toBeDefined(); + }); }); diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx index 2ed1fce8fddd..4bc42604adf3 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx @@ -1,10 +1,15 @@ import React, { useState } from 'react'; import { Hex } from '@metamask/utils'; -import { BorderColor } from '../../../../../helpers/constants/design-system'; +import { + BackgroundColor, + BorderColor, + BorderRadius, +} from '../../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { AvatarAccount, AvatarAccountSize, + Box, IconName, } from '../../../../component-library'; import { EditAccountsModal, EditNetworksModal } from '../../..'; @@ -55,13 +60,15 @@ export const SiteCell: React.FC = ({ selectedChainIds.includes(chainId), ); + const selectedChainIdsLength = selectedChainIds.length; + // Determine the messages for connected and not connected states const accountMessageConnectedState = selectedAccounts.length === 1 - ? t('connectedWithAccount', [ + ? t('connectedWithAccountName', [ selectedAccounts[0].label || selectedAccounts[0].metadata.name, ]) - : t('connectedWith'); + : t('connectedWithAccount', [accounts.length]); const accountMessageNotConnectedState = selectedAccounts.length === 1 ? t('requestingForAccount', [ @@ -71,36 +78,48 @@ export const SiteCell: React.FC = ({ return ( <> - setShowEditAccountsModal(true)} - content={ - // Why this difference? - selectedAccounts.length === 1 ? ( - - ) : ( - - ) - } - /> - setShowEditNetworksModal(true)} - content={} - /> - + + setShowEditAccountsModal(true)} + paddingBottomValue={2} + paddingTopValue={0} + content={ + // Why this difference? + selectedAccounts.length === 1 ? ( + + ) : ( + + ) + } + /> + setShowEditNetworksModal(true)} + paddingTopValue={2} + paddingBottomValue={0} + content={} + /> + {showEditAccountsModal && (
- -
-
-

- See your accounts and suggest transactions -

+
+
+

- Requesting for Test Account - + See your accounts and suggest transactions +

-
- -
-
-
- +
-

- Use your enabled networks -

+
+
+

- Requesting for - + Use your enabled networks +

Alerts"" - data-tooltipped="" - style="display: inline;" + class="mm-box mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--align-items-center" > + + Requesting for +
Alerts"" + data-tooltipped="" + style="display: inline;" >
- G +
+ G +
-
-
- Custom Mainnet RPC logo +
+ Custom Mainnet RPC logo +
+
-
diff --git a/ui/pages/permissions-connect/connect-page/connect-page.test.tsx b/ui/pages/permissions-connect/connect-page/connect-page.test.tsx index 9440d5031334..d7c50c6aa501 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.test.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.test.tsx @@ -69,7 +69,7 @@ describe('ConnectPage', () => { it('should render confirm and cancel button', () => { const { getByText } = render(); - const confirmButton = getByText('Confirm'); + const confirmButton = getByText('Connect'); const cancelButton = getByText('Cancel'); expect(confirmButton).toBeDefined(); expect(cancelButton).toBeDefined(); diff --git a/ui/pages/permissions-connect/connect-page/connect-page.tsx b/ui/pages/permissions-connect/connect-page/connect-page.tsx index f332ba6cc07e..a30047fbd38a 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.tsx @@ -24,13 +24,16 @@ import { } from '../../../components/multichain/pages/page'; import { SiteCell } from '../../../components/multichain/pages/review-permissions-page'; import { + BackgroundColor, BlockSize, Display, + FlexDirection, TextVariant, } from '../../../helpers/constants/design-system'; import { MergedInternalAccount } from '../../../selectors/selectors.types'; import { mergeAccounts } from '../../../components/multichain/account-list-menu/account-list-menu'; import { TEST_CHAINS } from '../../../../shared/constants/network'; +import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer'; export type ConnectPageRequest = { id: string; @@ -97,14 +100,15 @@ export const ConnectPage: React.FC = ({ return ( -
+
{t('connectWithMetaMask')} {t('connectionDescription')}:
- + = ({ />
- - - + + + + + +
diff --git a/ui/pages/permissions-connect/index.scss b/ui/pages/permissions-connect/index.scss index 513809505d50..954ec7a1121c 100644 --- a/ui/pages/permissions-connect/index.scss +++ b/ui/pages/permissions-connect/index.scss @@ -44,4 +44,8 @@ justify-self: flex-end; font-weight: bold; } + + .connect-page { + background-color: var(--color-background-alternative); // main-container adds the width but overrides the boxProps. So, we need extra class to apply css + } } diff --git a/ui/pages/routes/routes.component.js b/ui/pages/routes/routes.component.js index a02ecfa32ef9..82361cb6b690 100644 --- a/ui/pages/routes/routes.component.js +++ b/ui/pages/routes/routes.component.js @@ -564,6 +564,17 @@ export default class Routes extends Component { return true; } + const isReviewPermissionsPgae = Boolean( + matchPath(location.pathname, { + path: REVIEW_PERMISSIONS, + exact: false, + }), + ); + + if (isReviewPermissionsPgae) { + return true; + } + if (windowType === ENVIRONMENT_TYPE_POPUP && this.onConfirmPage()) { return true; } From 8b3556394db3fb5a9fe0398fe747686351b20b22 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 1 Oct 2024 11:05:37 +0200 Subject: [PATCH 09/14] fix: Allow state updates in Snaps interfaces to state values that are falsy (#27488) ## **Description** Fixes a problem where state updates to Snaps interfaces would be ignored if the value was falsy, e.g. empty string. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27488?quickstart=1) ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/e6ea3cee-f8f3-4f0d-9c48-f47ab8456b71 ### **After** https://github.com/user-attachments/assets/6ebf5124-dc16-4f6d-8d6c-bf46ae00c998 --- ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx | 2 +- ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx | 2 +- ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx | 2 +- ui/contexts/snaps/snap-interface.tsx | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx index c9000c13839b..f2cb85cc4ef0 100644 --- a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx +++ b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx @@ -34,7 +34,7 @@ export const SnapUIDropdown: FunctionComponent = ({ const [value, setValue] = useState(initialValue ?? ''); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setValue(initialValue); } }, [initialValue]); diff --git a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx index b6f68c646ec5..fbb340d92889 100644 --- a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx +++ b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx @@ -22,7 +22,7 @@ export const SnapUIInput: FunctionComponent< const [value, setValue] = useState(initialValue ?? ''); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setValue(initialValue); } }, [initialValue]); diff --git a/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx b/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx index 49a13400bbf8..a0869ff0b46b 100644 --- a/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx +++ b/ui/components/app/snaps/snap-ui-selector/snap-ui-selector.tsx @@ -102,7 +102,7 @@ export const SnapUISelector: React.FunctionComponent = ({ const [isModalOpen, setIsModalOpen] = useState(false); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setSelectedOption(initialValue); } }, [initialValue]); diff --git a/ui/contexts/snaps/snap-interface.tsx b/ui/contexts/snaps/snap-interface.tsx index 25249d31420a..7e37485c3ea2 100644 --- a/ui/contexts/snaps/snap-interface.tsx +++ b/ui/contexts/snaps/snap-interface.tsx @@ -230,7 +230,7 @@ export const SnapInterfaceContextProvider: FunctionComponent< ? (initialState[form] as FormState)?.[name] : (initialState as FormState)?.[name]; - if (value) { + if (value !== undefined && value !== null) { return value; } From 7fa528628368d6ad9d15aa48c7090ff37084e190 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 1 Oct 2024 12:28:53 +0200 Subject: [PATCH 10/14] fix(snaps): Keep focus on input if interface re-renders (#27429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR adds a reference to the currently focused input in a snap interface and set the focus back to the last focused input if the interface re-renders. This fixes a problem where it will loose input focus and set it to the last input of the interface if an interface was re-rendered. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27429?quickstart=1) ## **Related issues** Fixes: #27424 ## **Manual testing steps** 1. Go to test-snaps 2. Use the send flow example snap 3. Try typing something in the "To address" field 4. The focus should stay on your input. ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/4a15acda-f41b-4e32-bc71-dfceaa1920c9 ### **After** https://github.com/user-attachments/assets/80745da0-edbb-4ab1-8a32-d2b465a31aee ## **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/develop/.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/develop/.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. --- .../app/snaps/snap-ui-input/snap-ui-input.tsx | 23 +++++++++++++++++-- ui/contexts/snaps/snap-interface.tsx | 10 ++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx index fbb340d92889..51c1c9a54a7a 100644 --- a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx +++ b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx @@ -2,6 +2,7 @@ import React, { ChangeEvent, FunctionComponent, useEffect, + useRef, useState, } from 'react'; import { useSnapInterfaceContext } from '../../../../contexts/snaps'; @@ -15,7 +16,10 @@ export type SnapUIInputProps = { export const SnapUIInput: FunctionComponent< SnapUIInputProps & FormTextFieldProps<'div'> > = ({ name, form, ...props }) => { - const { handleInputChange, getValue } = useSnapInterfaceContext(); + const { handleInputChange, getValue, focusedInput, setCurrentFocusedInput } = + useSnapInterfaceContext(); + + const inputRef = useRef(null); const initialValue = getValue(name, form) as string; @@ -27,14 +31,29 @@ export const SnapUIInput: FunctionComponent< } }, [initialValue]); + /* + * Focus input if the last focused input was this input + * This avoids loosing the focus when the UI is re-rendered + */ + useEffect(() => { + if (inputRef.current && name === focusedInput) { + (inputRef.current.children[0] as HTMLInputElement).focus(); + } + }, [inputRef]); + const handleChange = (event: ChangeEvent) => { setValue(event.target.value); handleInputChange(name, event.target.value ?? null, form); }; + const handleFocus = () => setCurrentFocusedInput(name); + const handleBlur = () => setCurrentFocusedInput(null); + return ( void; +export type SetCurrentInputFocus = (name: string | null) => void; + export type SnapInterfaceContextType = { handleEvent: HandleEvent; getValue: GetValue; handleInputChange: HandleInputChange; handleFileChange: HandleFileChange; + setCurrentFocusedInput: SetCurrentInputFocus; + focusedInput: string | null; snapId: string; }; @@ -80,6 +84,7 @@ export const SnapInterfaceContextProvider: FunctionComponent< // UI. It's kept in a ref to avoid useless re-rendering of the entire tree of // components. const internalState = useRef(initialState ?? {}); + const focusedInput = useRef(null); // Since the internal state is kept in a reference, it won't update when the // interface is updated. We have to manually update it. @@ -237,6 +242,9 @@ export const SnapInterfaceContextProvider: FunctionComponent< return undefined; }; + const setCurrentFocusedInput: SetCurrentInputFocus = (name) => + (focusedInput.current = name); + return ( From 778e5ab5378f002966036b501c02d411bea432db Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Tue, 1 Oct 2024 12:58:40 +0100 Subject: [PATCH 11/14] fix: genUnapprovedApproveConfirmation import path (#27530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Merging https://github.com/MetaMask/metamask-extension/pull/27358 and then https://github.com/MetaMask/metamask-extension/pull/27391 without rebasing caused an erroneous import path in unit tests. This caused failing test and import linting. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27530?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/develop/.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/develop/.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. --- .../hooks/useLedgerConnection.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/pages/confirmations/hooks/useLedgerConnection.test.ts b/ui/pages/confirmations/hooks/useLedgerConnection.test.ts index 42868115a369..7041b11b1aa4 100644 --- a/ui/pages/confirmations/hooks/useLedgerConnection.test.ts +++ b/ui/pages/confirmations/hooks/useLedgerConnection.test.ts @@ -1,18 +1,18 @@ -import type { TransactionMeta } from '@metamask/transaction-controller'; import type { KeyringObject } from '@metamask/keyring-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import type { Hex } from '@metamask/utils'; import { cloneDeep } from 'lodash'; -import { KeyringType } from '../../../../shared/constants/keyring'; -import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers'; -import { getMockConfirmStateForTransaction } from '../../../../test/data/confirmations/helper'; -import { genUnapprovedApproveConfirmation } from '../../../../test/data/confirmations/contract-interaction'; -import { flushPromises } from '../../../../test/lib/timer-helpers'; import { + HardwareTransportStates, + LEDGER_USB_VENDOR_ID, LedgerTransportTypes, WebHIDConnectedStatuses, - LEDGER_USB_VENDOR_ID, - HardwareTransportStates, } from '../../../../shared/constants/hardware-wallets'; +import { KeyringType } from '../../../../shared/constants/keyring'; +import { getMockConfirmStateForTransaction } from '../../../../test/data/confirmations/helper'; +import { genUnapprovedApproveConfirmation } from '../../../../test/data/confirmations/token-approve'; +import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers'; +import { flushPromises } from '../../../../test/lib/timer-helpers'; import * as appActions from '../../../ducks/app/app'; import { attemptLedgerTransportCreation } from '../../../store/actions'; import useLedgerConnection from './useLedgerConnection'; From 874f704259535818ff62a69746a73c26f2fea495 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Tue, 1 Oct 2024 14:40:27 +0200 Subject: [PATCH 12/14] fix(NOTIFY-1171): account syncing performance and bug fixes (#27529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR fixes some bugs and adds as well some huge performance improvements for account syncing. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27529?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Create a new SRP 2. Add new accounts, rename some 3. Uninstall extension and reinstall 4. Import your previously created SRP 5. All your previously created accounts and respective names should be there! ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: MetaMask Bot --- lavamoat/browserify/beta/policy.json | 2 -- lavamoat/browserify/flask/policy.json | 2 -- lavamoat/browserify/main/policy.json | 2 -- lavamoat/browserify/mmi/policy.json | 2 -- package.json | 2 +- yarn.lock | 10 +++++----- 6 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 8c60013092de..2116f0a1a7c8 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -2131,12 +2131,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/package.json b/package.json index 86b0718ff463..e3063816cbcb 100644 --- a/package.json +++ b/package.json @@ -344,7 +344,7 @@ "@metamask/post-message-stream": "^8.0.0", "@metamask/ppom-validator": "0.34.0", "@metamask/preinstalled-example-snap": "^0.1.0", - "@metamask/profile-sync-controller": "^0.9.3", + "@metamask/profile-sync-controller": "^0.9.4", "@metamask/providers": "^14.0.2", "@metamask/queued-request-controller": "^2.0.0", "@metamask/rate-limit-controller": "^6.0.0", diff --git a/yarn.lock b/yarn.lock index 22889932623c..1434d86d4e62 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6053,9 +6053,9 @@ __metadata: languageName: node linkType: hard -"@metamask/profile-sync-controller@npm:^0.9.3": - version: 0.9.3 - resolution: "@metamask/profile-sync-controller@npm:0.9.3" +"@metamask/profile-sync-controller@npm:^0.9.4": + version: 0.9.4 + resolution: "@metamask/profile-sync-controller@npm:0.9.4" dependencies: "@metamask/base-controller": "npm:^7.0.1" "@metamask/keyring-api": "npm:^8.1.3" @@ -6071,7 +6071,7 @@ __metadata: "@metamask/accounts-controller": ^18.1.1 "@metamask/keyring-controller": ^17.2.0 "@metamask/snaps-controllers": ^9.7.0 - checksum: 10/31efea63cac0b5f01024163fb6911f971aeb6f7e7a7d71fa4a43b8e31e0fc60033e99bcfec19283f9410e7258bcd0ce3bf751bed374e6b3d09ea4a9782731320 + checksum: 10/86079da552eed316f2754bd899047de1d8d9d15d390c9cdee0aef66b95bea708b5c7929a8d8d946210cc0e4c52347fee971a5cf5130149d0ca60abdc85f47774 languageName: node linkType: hard @@ -26138,7 +26138,7 @@ __metadata: "@metamask/post-message-stream": "npm:^8.0.0" "@metamask/ppom-validator": "npm:0.34.0" "@metamask/preinstalled-example-snap": "npm:^0.1.0" - "@metamask/profile-sync-controller": "npm:^0.9.3" + "@metamask/profile-sync-controller": "npm:^0.9.4" "@metamask/providers": "npm:^14.0.2" "@metamask/queued-request-controller": "npm:^2.0.0" "@metamask/rate-limit-controller": "npm:^6.0.0" From cf55b09b190fc9fa0a5036b6f110b467d38f1274 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 1 Oct 2024 16:59:59 +0200 Subject: [PATCH 13/14] fix(snaps): Fix custom UI buttons submitting forms (#27531) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR fixes a bug in custom UI where a `Button` with no type would trigger a form submission when inside a form due to the type not being set by default. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27531?quickstart=1) ## **Related issues** Fixes: #27400 ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **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/develop/.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/develop/.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. --- ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx b/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx index cedcc375c17e..08fef2f9a6b7 100644 --- a/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx +++ b/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx @@ -23,7 +23,7 @@ export const SnapUIButton: FunctionComponent< > = ({ name, children, - type, + type = ButtonType.Button, variant = 'primary', disabled = false, className = '', From facf90562a5314d087fc9a1d2b17bae07f5097c7 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:03:01 +0200 Subject: [PATCH 14/14] feat: Enable gas included swaps (#27427) --- app/_locales/en/messages.json | 16 + app/scripts/controllers/swaps/swaps.test.ts | 2 + app/scripts/controllers/swaps/swaps.types.ts | 1 + shared/lib/swaps-utils.js | 2 + shared/lib/swaps-utils.test.js | 1 + test/jest/mock-store.js | 49 ++- ui/ducks/swaps/swaps.js | 31 +- ui/ducks/swaps/swaps.test.js | 25 +- ui/helpers/constants/zendesk-url.js | 2 + .../prepare-swap-page/prepare-swap-page.js | 11 +- .../swaps/prepare-swap-page/review-quote.js | 338 +++++++++++++----- .../prepare-swap-page/review-quote.test.js | 37 +- .../smart-transaction-status.js | 7 +- ui/pages/swaps/view-quote/view-quote.js | 5 +- ui/store/actions.ts | 10 +- 15 files changed, 424 insertions(+), 113 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index b880d92ad468..42a5a108fdf1 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2121,6 +2121,9 @@ "message": "This gas fee has been suggested by $1. Overriding this may cause a problem with your transaction. Please reach out to $1 if you have questions.", "description": "$1 represents the Dapp's origin" }, + "gasFee": { + "message": "Gas fee" + }, "gasIsETH": { "message": "Gas is $1 " }, @@ -2435,6 +2438,9 @@ "inYourSettings": { "message": "in your Settings" }, + "included": { + "message": "included" + }, "infuraBlockedNotification": { "message": "MetaMask is unable to connect to the blockchain host. Review possible reasons $1.", "description": "$1 is a clickable link with with text defined by the 'here' key" @@ -5668,12 +5674,22 @@ "message": "Gas fees are paid to crypto miners who process transactions on the $1 network. MetaMask does not profit from gas fees.", "description": "$1 is the selected network, e.g. Ethereum or BSC" }, + "swapGasIncludedTooltipExplanation": { + "message": "This quote incorporates gas fees by adjusting the token amount sent or received. You may receive ETH in a separate transaction on your activity list." + }, + "swapGasIncludedTooltipExplanationLinkText": { + "message": "Learn more about gas fees" + }, "swapHighSlippage": { "message": "High slippage" }, "swapHighSlippageWarning": { "message": "Slippage amount is very high." }, + "swapIncludesGasAndMetaMaskFee": { + "message": "Includes gas and a $1% MetaMask fee", + "description": "Provides information about the fee that metamask takes for swaps. $1 is a decimal number." + }, "swapIncludesMMFee": { "message": "Includes a $1% MetaMask fee.", "description": "Provides information about the fee that metamask takes for swaps. $1 is a decimal number." diff --git a/app/scripts/controllers/swaps/swaps.test.ts b/app/scripts/controllers/swaps/swaps.test.ts index 3fa4f1ff9409..4ed1b545f170 100644 --- a/app/scripts/controllers/swaps/swaps.test.ts +++ b/app/scripts/controllers/swaps/swaps.test.ts @@ -26,6 +26,7 @@ const MOCK_FETCH_PARAMS: FetchTradesInfoParams = { fromAddress: '0x7F18BB4Dd92CF2404C54CBa1A9BE4A1153bdb078', exchangeList: 'zeroExV1', balanceError: false, + enableGasIncludedQuotes: false, }; const TEST_AGG_ID_1 = 'TEST_AGG_1'; @@ -1164,6 +1165,7 @@ describe('SwapsController', function () { fromAddress: '', exchangeList: 'zeroExV1', balanceError: false, + enableGasIncludedQuotes: false, metaData: {} as FetchTradesInfoParamsMetadata, }; const swapsFeatureIsLive = false; diff --git a/app/scripts/controllers/swaps/swaps.types.ts b/app/scripts/controllers/swaps/swaps.types.ts index 44e4d4939742..ca059723277a 100644 --- a/app/scripts/controllers/swaps/swaps.types.ts +++ b/app/scripts/controllers/swaps/swaps.types.ts @@ -308,6 +308,7 @@ export type FetchTradesInfoParams = { fromAddress: string; exchangeList: string; balanceError: boolean; + enableGasIncludedQuotes: boolean; }; export type FetchTradesInfoParamsMetadata = { diff --git a/shared/lib/swaps-utils.js b/shared/lib/swaps-utils.js index d80d70902810..c51a3ac1198e 100644 --- a/shared/lib/swaps-utils.js +++ b/shared/lib/swaps-utils.js @@ -265,6 +265,7 @@ export async function fetchTradesInfo( value, fromAddress, exchangeList, + enableGasIncludedQuotes, }, { chainId }, ) { @@ -275,6 +276,7 @@ export async function fetchTradesInfo( slippage, timeout: SECOND * 10, walletAddress: fromAddress, + enableGasIncludedQuotes, }; if (exchangeList) { diff --git a/shared/lib/swaps-utils.test.js b/shared/lib/swaps-utils.test.js index 06080a8f55e7..891c1c5fb961 100644 --- a/shared/lib/swaps-utils.test.js +++ b/shared/lib/swaps-utils.test.js @@ -87,6 +87,7 @@ describe('Swaps Utils', () => { sourceDecimals: TOKENS[0].decimals, sourceTokenInfo: { ...TOKENS[0] }, destinationTokenInfo: { ...TOKENS[1] }, + enableGasIncludedQuotes: false, }, { chainId: CHAIN_IDS.MAINNET }, ); diff --git a/test/jest/mock-store.js b/test/jest/mock-store.js index 625b6dcf6c83..736b9c4eb325 100644 --- a/test/jest/mock-store.js +++ b/test/jest/mock-store.js @@ -210,7 +210,7 @@ export const createSwapsMockStore = () => { }, ], useCurrencyRateCheck: true, - currentCurrency: 'ETH', + currentCurrency: 'usd', currencyRates: { ETH: { conversionRate: 1, @@ -469,6 +469,23 @@ export const createSwapsMockStore = () => { decimals: 18, }, fee: 1, + isGasIncludedTrade: false, + approvalTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, + tradeTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, }, TEST_AGG_2: { trade: { @@ -503,6 +520,36 @@ export const createSwapsMockStore = () => { decimals: 18, }, fee: 1, + isGasIncludedTrade: false, + approvalTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, + tradeTxFees: { + feeEstimate: 42000000000000, + fees: [ + { + maxFeePerGas: 2310003200, + maxPriorityFeePerGas: 513154852, + tokenFees: [ + { + token: { + address: '0x6b175474e89094c44da98b954eedeac495271d0f', + symbol: 'DAI', + decimals: 18, + }, + balanceNeededToken: '0x426dc933c2e5a', + }, + ], + }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, }, }, fetchParams: { diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 8abb63aa4a14..e399dc663806 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -435,7 +435,14 @@ export const getPendingSmartTransactions = (state) => { }; export const getSmartTransactionFees = (state) => { - return state.metamask.smartTransactionsState?.fees; + const usedQuote = getUsedQuote(state); + if (!usedQuote?.isGasIncludedTrade) { + return state.metamask.smartTransactionsState?.fees; + } + return { + approvalTxFees: usedQuote.approvalTxFees, + tradeTxFees: usedQuote.tradeTxFees, + }; }; export const getSmartTransactionEstimatedGas = (state) => { @@ -780,6 +787,8 @@ export const fetchQuotesAndSetQuoteState = ( fromAddress: selectedAccount.address, balanceError, sourceDecimals: fromTokenDecimals, + enableGasIncludedQuotes: + currentSmartTransactionsEnabled && smartTransactionsOptInStatus, }, { sourceTokenInfo, @@ -933,6 +942,7 @@ export const signAndSendSwapsSmartTransaction = ({ stx_enabled: smartTransactionsEnabled, current_stx_enabled: currentSmartTransactionsEnabled, stx_user_opt_in: smartTransactionsOptInStatus, + is_gas_included_trade: usedQuote.isGasIncludedTrade, ...additionalTrackingParams, }; trackEvent({ @@ -964,13 +974,18 @@ export const signAndSendSwapsSmartTransaction = ({ value: '0x0', }; } - const fees = await dispatch( - fetchSwapsSmartTransactionFees({ - unsignedTransaction, - approveTxParams: updatedApproveTxParams, - fallbackOnNotEnoughFunds: true, - }), - ); + let fees; + if (usedQuote.isGasIncludedTrade) { + fees = getSmartTransactionFees(state); + } else { + fees = await dispatch( + fetchSwapsSmartTransactionFees({ + unsignedTransaction, + approveTxParams: updatedApproveTxParams, + fallbackOnNotEnoughFunds: true, + }), + ); + } if (!fees) { log.error('"fetchSwapsSmartTransactionFees" failed'); dispatch(setSwapsSTXSubmitLoading(false)); diff --git a/ui/ducks/swaps/swaps.test.js b/ui/ducks/swaps/swaps.test.js index 0bba5e5a68be..83c133572c0d 100644 --- a/ui/ducks/swaps/swaps.test.js +++ b/ui/ducks/swaps/swaps.test.js @@ -652,13 +652,36 @@ describe('Ducks - Swaps', () => { }); describe('getSmartTransactionFees', () => { - it('returns unsigned transactions and estimates', () => { + it('returns estimates from the STX controller', () => { const state = createSwapsMockStore(); const smartTransactionFees = swaps.getSmartTransactionFees(state); expect(smartTransactionFees).toMatchObject( state.metamask.smartTransactionsState.fees, ); }); + + it('returns estimates from a selected quote', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.quotes.TEST_AGG_2.isGasIncludedTrade = true; + const smartTransactionFees = swaps.getSmartTransactionFees(state); + expect(smartTransactionFees).toMatchObject({ + approvalTxFees: + state.metamask.swapsState.quotes.TEST_AGG_2.approvalTxFees, + tradeTxFees: state.metamask.swapsState.quotes.TEST_AGG_2.tradeTxFees, + }); + }); + + it('returns estimates from a top quote if no quote is selected', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.selectedAggId = null; + state.metamask.swapsState.quotes.TEST_AGG_BEST.isGasIncludedTrade = true; + const smartTransactionFees = swaps.getSmartTransactionFees(state); + expect(smartTransactionFees).toMatchObject({ + approvalTxFees: + state.metamask.swapsState.quotes.TEST_AGG_BEST.approvalTxFees, + tradeTxFees: state.metamask.swapsState.quotes.TEST_AGG_BEST.tradeTxFees, + }); + }); }); describe('getSmartTransactionEstimatedGas', () => { diff --git a/ui/helpers/constants/zendesk-url.js b/ui/helpers/constants/zendesk-url.js index 1e29bd9b1fc7..3986df5b41ef 100644 --- a/ui/helpers/constants/zendesk-url.js +++ b/ui/helpers/constants/zendesk-url.js @@ -8,6 +8,8 @@ const ZENDESK_URLS = { CUSTOMIZE_NONCE: 'https://support.metamask.io/transactions-and-gas/transactions/how-to-customize-a-transaction-nonce/', GAS_FEES: 'https://support.metamask.io/transactions-and-gas/gas-fees/', + SWAPS_GAS_FEES: + 'https://support.metamask.io/token-swaps/user-guide-swaps/#gas-fees', HARDWARE_CONNECTION: 'https://support.metamask.io/privacy-and-security/hardware-wallet-hub/', IMPORT_ACCOUNTS: diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js index 98bb6933d0c3..7ea900c5eb59 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js @@ -782,10 +782,17 @@ export default function PrepareSwapPage({ ); } + const isNonDefaultToken = !isSwapsDefaultTokenSymbol( + fromTokenSymbol, + chainId, + ); + const hasPositiveFromTokenBalance = rawFromTokenBalance > 0; + const isTokenEligibleForMaxBalance = + isSmartTransaction || (!isSmartTransaction && isNonDefaultToken); const showMaxBalanceLink = fromTokenSymbol && - !isSwapsDefaultTokenSymbol(fromTokenSymbol, chainId) && - rawFromTokenBalance > 0; + isTokenEligibleForMaxBalance && + hasPositiveFromTokenBalance; return (
diff --git a/ui/pages/swaps/prepare-swap-page/review-quote.js b/ui/pages/swaps/prepare-swap-page/review-quote.js index 496ae5ee6d9e..31cf9959f231 100644 --- a/ui/pages/swaps/prepare-swap-page/review-quote.js +++ b/ui/pages/swaps/prepare-swap-page/review-quote.js @@ -23,7 +23,6 @@ import { useGasFeeInputs } from '../../confirmations/hooks/useGasFeeInputs'; import { MetaMetricsContext } from '../../../contexts/metametrics'; import { getQuotes, - getSelectedQuote, getApproveTxParams, getFetchParams, setBalanceError, @@ -36,6 +35,7 @@ import { getDestinationTokenInfo, getUsedSwapsGasPrice, getTopQuote, + getUsedQuote, signAndSendTransactions, getBackgroundSwapRouteState, swapsQuoteSelected, @@ -84,6 +84,7 @@ import { decimalToHex, decWEIToDecETH, sumHexes, + hexToDecimal, } from '../../../../shared/modules/conversion.utils'; import { getCustomTxParamsData } from '../../confirmations/confirm-approve/confirm-approve.util'; import { @@ -113,6 +114,7 @@ import { Size, FlexDirection, Severity, + FontStyle, } from '../../../helpers/constants/design-system'; import { BannerAlert, @@ -143,11 +145,41 @@ import { import ExchangeRateDisplay from '../exchange-rate-display'; import InfoTooltip from '../../../components/ui/info-tooltip'; import useRamps from '../../../hooks/ramps/useRamps/useRamps'; +import { getTokenFiatAmount } from '../../../helpers/utils/token-util'; +import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import ViewQuotePriceDifference from './view-quote-price-difference'; import SlippageNotificationModal from './slippage-notification-modal'; let intervalId; +const ViewAllQuotesLink = React.memo(function ViewAllQuotesLink({ + trackAllAvailableQuotesOpened, + setSelectQuotePopoverShown, + t, +}) { + const handleClick = useCallback(() => { + trackAllAvailableQuotesOpened(); + setSelectQuotePopoverShown(true); + }, [trackAllAvailableQuotesOpened, setSelectQuotePopoverShown]); + + return ( + + {t('viewAllQuotes')} + + ); +}); + +ViewAllQuotesLink.propTypes = { + trackAllAvailableQuotesOpened: PropTypes.func.isRequired, + setSelectQuotePopoverShown: PropTypes.func.isRequired, + t: PropTypes.func.isRequired, +}; + export default function ReviewQuote({ setReceiveToAmount }) { const history = useHistory(); const dispatch = useDispatch(); @@ -206,9 +238,8 @@ export default function ReviewQuote({ setReceiveToAmount }) { const balanceError = useSelector(getBalanceError); const fetchParams = useSelector(getFetchParams, isEqual); const approveTxParams = useSelector(getApproveTxParams, shallowEqual); - const selectedQuote = useSelector(getSelectedQuote, isEqual); const topQuote = useSelector(getTopQuote, isEqual); - const usedQuote = selectedQuote || topQuote; + const usedQuote = useSelector(getUsedQuote, isEqual); const tradeValue = usedQuote?.trade?.value ?? '0x0'; const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); const chainId = useSelector(getCurrentChainId); @@ -229,6 +260,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { const smartTransactionFees = useSelector(getSmartTransactionFees, isEqual); const swapsNetworkConfig = useSelector(getSwapsNetworkConfig, shallowEqual); const unsignedTransaction = usedQuote.trade; + const { isGasIncludedTrade } = usedQuote; const isSmartTransaction = currentSmartTransactionsEnabled && smartTransactionsOptInStatus; @@ -880,7 +912,9 @@ export default function ReviewQuote({ setReceiveToAmount }) { ]); useEffect(() => { - if (isSmartTransaction && !insufficientTokens) { + // If it's a smart transaction, has sufficient tokens, and gas is not included in the trade, + // set up gas fee polling. + if (isSmartTransaction && !insufficientTokens && !isGasIncludedTrade) { const unsignedTx = { from: unsignedTransaction.from, to: unsignedTransaction.to, @@ -923,6 +957,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { chainId, swapsNetworkConfig.stxGetTransactionsRefreshTime, insufficientTokens, + isGasIncludedTrade, ]); useEffect(() => { @@ -1045,6 +1080,40 @@ export default function ReviewQuote({ setReceiveToAmount }) { } }; + const gasTokenFiatAmount = useMemo(() => { + if (!isGasIncludedTrade) { + return undefined; + } + const tradeTxTokenFee = + smartTransactionFees?.tradeTxFees?.fees?.[0]?.tokenFees?.[0]; + if (!tradeTxTokenFee) { + return undefined; + } + const { token: { address, decimals, symbol } = {}, balanceNeededToken } = + tradeTxTokenFee; + const checksumAddress = toChecksumHexAddress(address); + const contractExchangeRate = memoizedTokenConversionRates[checksumAddress]; + const gasTokenAmountDec = calcTokenAmount( + hexToDecimal(balanceNeededToken), + decimals, + ).toString(10); + return getTokenFiatAmount( + contractExchangeRate, + conversionRate, + currentCurrency, + gasTokenAmountDec, + symbol, + true, + true, + ); + }, [ + isGasIncludedTrade, + smartTransactionFees, + memoizedTokenConversionRates, + conversionRate, + currentCurrency, + ]); + return (
@@ -1122,9 +1191,9 @@ export default function ReviewQuote({ setReceiveToAmount }) { - {t('quoteRate')} + {t('quoteRate')}* - + {isGasIncludedTrade && ( - - {t('transactionDetailGasHeading')} - - - {t('swapGasFeesExplanation', [ + + {t('gasFee')} + + +

+ {t('swapGasIncludedTooltipExplanation')} +

{ trackEvent({ - event: 'Clicked "Gas Fees: Learn More" Link', + event: + 'Clicked "GasIncluded tooltip: Learn More" Link', category: MetaMetricsEventCategory.Swaps, }); }} > - {t('swapGasFeesExplanationLinkText')} - , - ])} -

- } - /> + {t('swapGasIncludedTooltipExplanationLinkText')} + + + } + /> +
+ + + {gasTokenFiatAmount} + + + {t('included')} + +
+ )} + {!isGasIncludedTrade && ( - - {feeInEth} - - + {t('transactionDetailGasHeading')} + + + {t('swapGasFeesExplanation', [ + { + trackEvent({ + event: 'Clicked "Gas Fees: Learn More" Link', + category: MetaMetricsEventCategory.Swaps, + }); + }} + > + {t('swapGasFeesExplanationLinkText')} + , + ])} +

+ } + /> +
+ - {` ${feeInFiat}`} - + + {feeInEth} + + + {` ${feeInFiat}`} + + - - {(maxFeeInFiat || maxFeeInEth) && ( + )} + {!isGasIncludedTrade && (maxFeeInFiat || maxFeeInEth) && ( @@ -1248,7 +1395,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { {t('swapEnableTokenForSwapping', [tokenApprovalTextComponent])} @@ -1264,32 +1411,55 @@ export default function ReviewQuote({ setReceiveToAmount }) { )} - - - {t('swapIncludesMetaMaskFeeViewAllQuotes', [ - metaMaskFee, - { - trackAllAvailableQuotesOpened(); - setSelectQuotePopoverShown(true); + {isGasIncludedTrade && ( + + + * {t('swapIncludesGasAndMetaMaskFee', [metaMaskFee])} + + + + + + )} + {!isGasIncludedTrade && ( + + + * + {t('swapIncludesMetaMaskFeeViewAllQuotes', [ + metaMaskFee, + - {t('viewAllQuotes')} - , - ])} - - + setSelectQuotePopoverShown={setSelectQuotePopoverShown} + t={t} + />, + ])} + + + )}
{ const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee –')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -73,7 +74,7 @@ describe('ReviewQuote', () => { const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee –')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -96,7 +97,7 @@ describe('ReviewQuote', () => { const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee –')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -106,4 +107,34 @@ describe('ReviewQuote', () => { expect(getByText('Edit limit')).toBeInTheDocument(); expect(getByText('Swap')).toBeInTheDocument(); }); + + it('renders the component with gas included quotes', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.quotes.TEST_AGG_2.isGasIncludedTrade = true; + state.metamask.marketData[CHAIN_IDS.MAINNET][ + '0x6B175474E89094C44Da98b954EedeAC495271d0F' // DAI token contract address. + ] = { + price: 2, + contractPercentChange1d: 0.004, + priceChange1d: 0.00004, + }; + state.metamask.currencyRates.ETH = { + conversionDate: 1708532473.416, + conversionRate: 2918.02, + usdConversionRate: 2918.02, + }; + const store = configureMockStore(middleware)(state); + const props = createProps(); + const { getByText } = renderWithProvider(, store); + expect(getByText('New quotes in')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); + expect( + getByText('* Includes gas and a 1% MetaMask fee'), + ).toBeInTheDocument(); + expect(getByText('view all quotes')).toBeInTheDocument(); + expect(getByText('Gas fee')).toBeInTheDocument(); + // $6.82 gas fee is calculated based on params set in the the beginning of the test. + expect(getByText('$6.82')).toBeInTheDocument(); + expect(getByText('Swap')).toBeInTheDocument(); + }); }); diff --git a/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js b/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js index 157190687f31..530372105b69 100644 --- a/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js +++ b/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js @@ -8,11 +8,10 @@ import { getFetchParams, prepareToLeaveSwaps, getCurrentSmartTransactions, - getSelectedQuote, - getTopQuote, getCurrentSmartTransactionsEnabled, getSwapsNetworkConfig, cancelSwapsSmartTransaction, + getUsedQuote, } from '../../../ducks/swaps/swaps'; import { isHardwareWallet, @@ -74,9 +73,7 @@ export default function SmartTransactionStatusPage() { const hardwareWalletUsed = useSelector(isHardwareWallet); const hardwareWalletType = useSelector(getHardwareWalletType); const needsTwoConfirmations = true; - const selectedQuote = useSelector(getSelectedQuote, isEqual); - const topQuote = useSelector(getTopQuote, isEqual); - const usedQuote = selectedQuote || topQuote; + const usedQuote = useSelector(getUsedQuote, isEqual); const currentSmartTransactions = useSelector( getCurrentSmartTransactions, isEqual, diff --git a/ui/pages/swaps/view-quote/view-quote.js b/ui/pages/swaps/view-quote/view-quote.js index 8dc17ac3c765..be02ba840eb5 100644 --- a/ui/pages/swaps/view-quote/view-quote.js +++ b/ui/pages/swaps/view-quote/view-quote.js @@ -23,7 +23,6 @@ import { MetaMetricsContext } from '../../../contexts/metametrics'; import FeeCard from '../fee-card'; import { getQuotes, - getSelectedQuote, getApproveTxParams, getFetchParams, setBalanceError, @@ -36,6 +35,7 @@ import { getDestinationTokenInfo, getUsedSwapsGasPrice, getTopQuote, + getUsedQuote, signAndSendTransactions, getBackgroundSwapRouteState, swapsQuoteSelected, @@ -181,9 +181,8 @@ export default function ViewQuote() { const balanceError = useSelector(getBalanceError); const fetchParams = useSelector(getFetchParams, isEqual); const approveTxParams = useSelector(getApproveTxParams, shallowEqual); - const selectedQuote = useSelector(getSelectedQuote, isEqual); const topQuote = useSelector(getTopQuote, isEqual); - const usedQuote = selectedQuote || topQuote; + const usedQuote = useSelector(getUsedQuote, isEqual); const tradeValue = usedQuote?.trade?.value ?? '0x0'; const swapsQuoteRefreshTime = useSelector(getSwapsQuoteRefreshTime); const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); diff --git a/ui/store/actions.ts b/ui/store/actions.ts index c4bed2665a6b..dae0052c46f6 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -3664,6 +3664,7 @@ export function fetchAndSetQuotes( fromAddress: string; balanceError: string; sourceDecimals: number; + enableGasIncludedQuotes: boolean; }, fetchParamsMetaData: { sourceTokenInfo: Token; @@ -4757,18 +4758,15 @@ export function signAndSendSmartTransaction({ unsignedTransaction, smartTransactionFees.fees, ); - const signedCanceledTransactions = await createSignedTransactions( - unsignedTransaction, - smartTransactionFees.cancelFees, - true, - ); try { const response = await submitRequestToBackground<{ uuid: string }>( 'submitSignedTransactions', [ { signedTransactions, - signedCanceledTransactions, + // The "signedCanceledTransactions" parameter is still expected by the STX controller but is no longer used. + // So we are passing an empty array. The parameter may be deprecated in a future update. + signedCanceledTransactions: [], txParams: unsignedTransaction, }, ],