From d06dad70d8ade01aa569280cfddd05da24bfaa54 Mon Sep 17 00:00:00 2001 From: Jony Bursztyn Date: Thu, 19 Dec 2024 12:18:54 +0000 Subject: [PATCH 1/7] feat: add metametrics events to carousel (#29141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces tracking metrics for banners in the carousel to monitor their effectiveness. The following events are added: - **Banner selected:** Triggered when a banner or a button within a banner is clicked. - **Close all banners:** Triggered when the last banner in the carousel is closed. - **Banner shown:** Triggered when a banner is displayed in the carousel. ### Tracking Implementation Details: #### Banner selected When a banner or button in a banner is clicked: ```javascript trackEvent({ event: MetaMetricsEventName.BannerSelect, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` #### Close all banners When the last banner in the carousel is closed: ```javascript trackEvent({ event: MetaMetricsEventName.BannerCloseAll, category: MetaMetricsEventCategory.Banner }); ``` #### Banner shown When a banner is displayed in the carousel: ```javascript trackEvent({ event: MetaMetricsEventName.BannerDisplay, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3764 ## **Manual testing steps** 1. Open the carousel. 2. Click on banners or buttons within banners to trigger the "Banner selected" event. 3. Close the last banner to trigger the "Close all banners" event. 4. Navigate through the carousel to ensure the "Banner shown" event is fired for each displayed banner. ## **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/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- shared/constants/metametrics.ts | 4 ++ .../account-overview-layout.tsx | 51 +++++++++++++++--- .../multichain/carousel/carousel.test.tsx | 53 +++++++++++++++++-- .../multichain/carousel/carousel.tsx | 16 +++++- .../multichain/carousel/carousel.types.ts | 3 +- 5 files changed, 114 insertions(+), 13 deletions(-) diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index 9b99140b7d24..8b59e95e650c 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -645,6 +645,9 @@ export enum MetaMetricsEventName { AppUnlockedFailed = 'App Unlocked Failed', AppLocked = 'App Locked', AppWindowExpanded = 'App Window Expanded', + BannerDisplay = 'Banner Display', + BannerCloseAll = 'Banner Close All', + BannerSelect = 'Banner Select', BridgeLinkClicked = 'Bridge Link Clicked', BitcoinSupportToggled = 'Bitcoin Support Toggled', BitcoinTestnetSupportToggled = 'Bitcoin Testnet Support Toggled', @@ -912,6 +915,7 @@ export enum MetaMetricsEventCategory { App = 'App', Auth = 'Auth', Background = 'Background', + Banner = 'Banner', // The TypeScript ESLint rule is incorrectly marking this line. /* eslint-disable-next-line @typescript-eslint/no-shadow */ Error = 'Error', diff --git a/ui/components/multichain/account-overview/account-overview-layout.tsx b/ui/components/multichain/account-overview/account-overview-layout.tsx index 01396b77d2c1..69fc2297ffea 100644 --- a/ui/components/multichain/account-overview/account-overview-layout.tsx +++ b/ui/components/multichain/account-overview/account-overview-layout.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useContext, useState, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import { isEqual } from 'lodash'; @@ -16,6 +16,12 @@ import { ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import useBridging from '../../../hooks/bridge/useBridging'; ///: END:ONLY_INCLUDE_IF +import { MetaMetricsContext } from '../../../contexts/metametrics'; +import { + MetaMetricsEventName, + MetaMetricsEventCategory, +} from '../../../../shared/constants/metametrics'; +import type { CarouselSlide } from '../../../../shared/constants/app-state'; import { AccountOverviewTabsProps, AccountOverviewTabs, @@ -42,6 +48,8 @@ export const AccountOverviewLayout = ({ const slides = useSelector(getSlides); const totalBalance = useSelector(getSelectedAccountCachedBalance); const isLoading = useSelector(getAppIsLoading); + const trackEvent = useContext(MetaMetricsContext); + const [hasRendered, setHasRendered] = useState(false); ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); @@ -76,8 +84,8 @@ export const AccountOverviewLayout = ({ dispatch(updateSlides(defaultSlides)); }, [hasZeroBalance]); - ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const handleCarouselClick = (id: string) => { + ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) if (id === 'bridge') { openBridgeExperience( 'Carousel', @@ -85,26 +93,57 @@ export const AccountOverviewLayout = ({ location.pathname.includes('asset') ? '&token=native' : '', ); } + ///: END:ONLY_INCLUDE_IF + + trackEvent({ + event: MetaMetricsEventName.BannerSelect, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: id, + }, + }); }; - ///: END:ONLY_INCLUDE_IF - const handleRemoveSlide = (id: string) => { + const handleRemoveSlide = (isLastSlide: boolean, id: string) => { if (id === 'fund' && hasZeroBalance) { return; } + if (isLastSlide) { + trackEvent({ + event: MetaMetricsEventName.BannerCloseAll, + category: MetaMetricsEventCategory.Banner, + }); + } dispatch(removeSlide(id)); }; + const handleRenderSlides = useCallback( + (renderedSlides: CarouselSlide[]) => { + if (!hasRendered) { + renderedSlides.forEach((slide) => { + trackEvent({ + event: MetaMetricsEventName.BannerDisplay, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: slide.id, + }, + }); + }); + setHasRendered(true); + } + }, + [hasRendered, trackEvent], + ); + return ( <>
{children}
diff --git a/ui/components/multichain/carousel/carousel.test.tsx b/ui/components/multichain/carousel/carousel.test.tsx index 71b9e4f4faa8..25fe8b562600 100644 --- a/ui/components/multichain/carousel/carousel.test.tsx +++ b/ui/components/multichain/carousel/carousel.test.tsx @@ -3,6 +3,40 @@ import { render, fireEvent } from '@testing-library/react'; import { Carousel } from './carousel'; import { MARGIN_VALUES, WIDTH_VALUES } from './constants'; +jest.mock('react-responsive-carousel', () => ({ + Carousel: ({ + children, + onChange, + }: { + children: React.ReactNode; + onChange?: (index: number) => void; + }) => ( +
+ {children} +
+
+
+ ), +})); + +jest.mock('react-redux', () => ({ + useSelector: jest.fn(), + useDispatch: () => jest.fn(), +})); + +jest.mock('reselect', () => ({ + createSelector: jest.fn(), + createDeepEqualSelector: jest.fn(), + createSelectorCreator: jest.fn(() => jest.fn()), + lruMemoize: jest.fn(), +})); + +jest.mock('../../../selectors/approvals', () => ({ + selectPendingApproval: jest.fn(), +})); + jest.mock('../../../hooks/useI18nContext', () => ({ useI18nContext: () => (key: string) => key, })); @@ -46,13 +80,24 @@ describe('Carousel', () => { expect(closeButtons).toHaveLength(2); fireEvent.click(closeButtons[0]); - expect(mockOnClose).toHaveBeenCalledWith('1'); + expect(mockOnClose).toHaveBeenCalledWith(false, '1'); const remainingSlides = mockSlides.filter((slide) => slide.id !== '1'); rerender(); - const updatedSlides = container.querySelectorAll('.mm-carousel-slide'); - expect(updatedSlides).toHaveLength(1); + const updatedCloseButtons = container.querySelectorAll( + '.mm-carousel-slide__close-button', + ); + expect(updatedCloseButtons).toHaveLength(1); + + fireEvent.click(updatedCloseButtons[0]); + expect(mockOnClose).toHaveBeenCalledWith(true, '2'); + + const finalSlides = remainingSlides.filter((slide) => slide.id !== '2'); + rerender(); + + const finalSlideElements = container.querySelectorAll('.mm-carousel-slide'); + expect(finalSlideElements).toHaveLength(0); }); it('should handle slide navigation', () => { @@ -65,7 +110,7 @@ describe('Carousel', () => { fireEvent.click(dots[1]); const slides = container.querySelectorAll('.mm-carousel-slide'); - expect(slides[1].parentElement).toHaveClass('selected'); + expect(slides[1].parentElement).toHaveClass('mock-carousel'); }); it('should return null when no slides are present', () => { diff --git a/ui/components/multichain/carousel/carousel.tsx b/ui/components/multichain/carousel/carousel.tsx index 3fbbe955a8eb..83423948c530 100644 --- a/ui/components/multichain/carousel/carousel.tsx +++ b/ui/components/multichain/carousel/carousel.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { Carousel as ResponsiveCarousel } from 'react-responsive-carousel'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { Box, BoxProps, BannerBase } from '../../component-library'; @@ -24,6 +24,7 @@ export const Carousel = React.forwardRef( isLoading = false, onClose, onClick, + onRenderSlides, ...props }: CarouselProps, ref: React.Ref, @@ -44,6 +45,17 @@ export const Carousel = React.forwardRef( }) .slice(0, MAX_SLIDES); + useEffect(() => { + if ( + visibleSlides && + visibleSlides.length > 0 && + onRenderSlides && + !isLoading + ) { + onRenderSlides(visibleSlides); + } + }, [visibleSlides, onRenderSlides, isLoading]); + const handleClose = (e: React.MouseEvent, slideId: string) => { e.preventDefault(); e.stopPropagation(); @@ -65,7 +77,7 @@ export const Carousel = React.forwardRef( setSelectedIndex(newSelectedIndex); if (onClose) { - onClose(slideId); + onClose(visibleSlides.length === 1, slideId); } }; diff --git a/ui/components/multichain/carousel/carousel.types.ts b/ui/components/multichain/carousel/carousel.types.ts index a8aef8df4839..3a6289e76a4b 100644 --- a/ui/components/multichain/carousel/carousel.types.ts +++ b/ui/components/multichain/carousel/carousel.types.ts @@ -3,6 +3,7 @@ import { CarouselSlide } from '../../../../shared/constants/app-state'; export type CarouselProps = { slides: CarouselSlide[]; isLoading?: boolean; - onClose?: (id: string) => void; + onClose?: (isLastSlide: boolean, id: string) => void; onClick?: (id: string) => void; + onRenderSlides?: (slides: CarouselSlide[]) => void; }; From e8d6765952d311e73cc08ca270326e2b46650701 Mon Sep 17 00:00:00 2001 From: Nidhi Kumari Date: Thu, 19 Dec 2024 12:20:17 +0000 Subject: [PATCH 2/7] fix: updated margin for import token banner (#29283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is to update the margin bottom in the detected token banner ## **Description** This PR updates the Top right bottom and left margin to 16px, 16px, 4px and 16px for import token banner ## **Related issues** Fixes: #26670 ## **Manual testing steps** 1. Must be a new user with not all tokens added 2. Open MetaMask 3. If user has detected tokens, check the margin of the banner ## **Screenshots/Recordings** ### **Before** ![Screenshot 2024-12-17 at 3 06 39 PM](https://github.com/user-attachments/assets/b912aa7a-04da-4354-88bc-c37ca4433b25) ### **After** ![Screenshot 2024-12-17 at 3 38 33 PM](https://github.com/user-attachments/assets/b7d4afba-615a-4df3-8598-ef2b82d18143) ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- ui/components/app/assets/asset-list/asset-list.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/components/app/assets/asset-list/asset-list.tsx b/ui/components/app/assets/asset-list/asset-list.tsx index 3cd06ec4686b..19d9ecdd4c0d 100644 --- a/ui/components/app/assets/asset-list/asset-list.tsx +++ b/ui/components/app/assets/asset-list/asset-list.tsx @@ -127,6 +127,7 @@ const AssetList = ({ onClickAsset, showTokensLinks }: AssetListProps) => { className="" actionButtonOnClick={() => setShowDetectedTokens(true)} margin={4} + marginBottom={1} /> ) : null} From 398ec979ef0ae27214e11ec644f6415df404a897 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 19 Dec 2024 10:08:56 -0330 Subject: [PATCH 3/7] ci: Improve accuracy of `wait-for-circleci-workflow-status` (#29310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The GitHub Actions workflow `wait-for-circleci-workflow-status` has been updated to ensure that it waits for the correct workflow. Previously it always chose the most recent workflow for the given branch, but it may have chosen a workflow corresponding to the wrong commit. It has been updated to find one matching the same commit that triggered the GitHub Actions workflow. A log has been added to help diagnose any future problems with this workflow, and to help with verification. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29310?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** Check the logs of the "Wait for CircleCI workflow status" job, and see that the workflow ID it's waiting on is correct when making multiple successive commits (comparing the timing using the CircleCI dashboard) Unfortunately the ID logged in the action is not shown on the CircleCI UI, but you can download the pipeline data with this command: `curl 'https://circleci.com/api/v2/project/gh/MetaMask/metamask-extension/pipeline?branch=[branch]' > pipeline.json` and look through the `pipeline.json` file for an entry that matches the logged ID. The `number` field is the pipeline number, which is in the CircleCI workflow URL (e.g. `https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/[pipeline number]/workflows/[some other number]`) ## **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/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .github/workflows/wait-for-circleci-workflow-status.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/wait-for-circleci-workflow-status.yml b/.github/workflows/wait-for-circleci-workflow-status.yml index 18e5ef7825d5..725a4c3b975d 100644 --- a/.github/workflows/wait-for-circleci-workflow-status.yml +++ b/.github/workflows/wait-for-circleci-workflow-status.yml @@ -13,8 +13,13 @@ jobs: OWNER: ${{ github.repository_owner }} REPOSITORY: ${{ github.event.repository.name }} BRANCH: ${{ github.head_ref || github.ref_name }} + # For a `push` event, the HEAD commit hash is `github.sha`. + # For a `pull_request` event, `github.sha` is instead the base branch commit hash. The + # HEAD commit hash is `pull_request.head.sha`. + HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha || github.sha }} run: | - pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items[0].id") + pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id") + echo "Waiting for pipeline '${pipeline_id}' for commit hash '${HEAD_COMMIT_HASH}'" workflow_status=$(curl --silent "https://circleci.com/api/v2/pipeline/$pipeline_id/workflow" | jq -r ".items[0].status") if [ "$workflow_status" == "running" ]; then From cf6a43d54e5dea8c0694d048fddf44cbb16f0c8e Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:24:07 +0100 Subject: [PATCH 4/7] chore: bump `@metamask/smart-transactions-controller` to `16.0.0` (#29344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps `@metamask/smart-transactions-controller` to `16.0.0` [CHANGELOG](https://github.com/MetaMask/smart-transactions-controller/blob/main/CHANGELOG.md#1600) - `@metamask/transaction-controller` has been bumped to `42.0.0` which match the current version used in the client. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29344?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** ## **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/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- package.json | 2 +- yarn.lock | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 1553c739936f..21ba753e2567 100644 --- a/package.json +++ b/package.json @@ -342,7 +342,7 @@ "@metamask/scure-bip39": "^2.0.3", "@metamask/selected-network-controller": "^19.0.0", "@metamask/signature-controller": "^23.1.0", - "@metamask/smart-transactions-controller": "^15.0.0", + "@metamask/smart-transactions-controller": "^16.0.0", "@metamask/snaps-controllers": "^9.15.0", "@metamask/snaps-execution-environments": "^6.10.0", "@metamask/snaps-rpc-methods": "^11.7.0", diff --git a/yarn.lock b/yarn.lock index 494ffcc5b085..2427563e548a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6188,9 +6188,9 @@ __metadata: languageName: node linkType: hard -"@metamask/smart-transactions-controller@npm:^15.0.0": - version: 15.0.0 - resolution: "@metamask/smart-transactions-controller@npm:15.0.0" +"@metamask/smart-transactions-controller@npm:^16.0.0": + version: 16.0.0 + resolution: "@metamask/smart-transactions-controller@npm:16.0.0" dependencies: "@babel/runtime": "npm:^7.24.1" "@ethereumjs/tx": "npm:^5.2.1" @@ -6202,18 +6202,17 @@ __metadata: "@metamask/eth-query": "npm:^4.0.0" "@metamask/polling-controller": "npm:^12.0.0" bignumber.js: "npm:^9.0.1" - events: "npm:^3.3.0" fast-json-patch: "npm:^3.1.0" lodash: "npm:^4.17.21" peerDependencies: "@metamask/network-controller": ^22.0.0 - "@metamask/transaction-controller": ^38.0.0 + "@metamask/transaction-controller": ^42.0.0 peerDependenciesMeta: "@metamask/accounts-controller": optional: true "@metamask/approval-controller": optional: true - checksum: 10/e30faad97451ba003da5b650040c0803d229954b6d6a28bc759ef6129d8e33f02dc2219c8346ae5049e62a84b62d8df211a403bdc80d59c1caed4ba182b6cc0a + checksum: 10/5bfe2e459ca75b3de31f28213e908f389a6a7e82b45dd43ee7fd4c46a619a561da030f7f90426af03063ff5dcfc90269a155230db484a2c415db11f8ad8caa02 languageName: node linkType: hard @@ -26609,7 +26608,7 @@ __metadata: "@metamask/scure-bip39": "npm:^2.0.3" "@metamask/selected-network-controller": "npm:^19.0.0" "@metamask/signature-controller": "npm:^23.1.0" - "@metamask/smart-transactions-controller": "npm:^15.0.0" + "@metamask/smart-transactions-controller": "npm:^16.0.0" "@metamask/snaps-controllers": "npm:^9.15.0" "@metamask/snaps-execution-environments": "npm:^6.10.0" "@metamask/snaps-rpc-methods": "npm:^11.7.0" From 927ef8c3b19da8ade4462e6adbd0e3e4cb8ca32c Mon Sep 17 00:00:00 2001 From: Alejandro Garcia Anglada Date: Thu, 19 Dec 2024 15:53:53 +0100 Subject: [PATCH 5/7] feat: bump solana snap (#29350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Solana snap bump https://github.com/MetaMask/snap-solana-wallet/releases/tag/v1.0.4 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29350?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/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Antonio Regadas Co-authored-by: Javier --- package.json | 2 +- privacy-snapshot.json | 2 ++ yarn.lock | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 21ba753e2567..71373b3f12c7 100644 --- a/package.json +++ b/package.json @@ -348,7 +348,7 @@ "@metamask/snaps-rpc-methods": "^11.7.0", "@metamask/snaps-sdk": "^6.13.0", "@metamask/snaps-utils": "^8.6.1", - "@metamask/solana-wallet-snap": "^1.0.3", + "@metamask/solana-wallet-snap": "^1.0.4", "@metamask/transaction-controller": "^42.0.0", "@metamask/user-operation-controller": "^21.0.0", "@metamask/utils": "^10.0.1", diff --git a/privacy-snapshot.json b/privacy-snapshot.json index fc5dafb7333c..24c6e9ae27da 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -3,6 +3,7 @@ "accounts.api.cx.metamask.io", "acl.execution.metamask.io", "api.blockchair.com", + "api.devnet.solana.com", "api.lens.dev", "api.segment.io", "api.web3modal.com", @@ -59,6 +60,7 @@ "sepolia.infura.io", "signature-insights.api.cx.metamask.io", "snaps.metamask.io", + "solana.rpc.grove.city", "sourcify.dev", "start.metamask.io", "static.cx.metamask.io", diff --git a/yarn.lock b/yarn.lock index 2427563e548a..d10097abd78e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6346,10 +6346,10 @@ __metadata: languageName: node linkType: hard -"@metamask/solana-wallet-snap@npm:^1.0.3": - version: 1.0.3 - resolution: "@metamask/solana-wallet-snap@npm:1.0.3" - checksum: 10/4c7c0f05676e7bb84140226c1a3bd716493a6f3582142de83575df682e8351c7583fc5db6209fbde1b43f376fd4eda4d2063f0e651d8209e92001514fc8caf81 +"@metamask/solana-wallet-snap@npm:^1.0.4": + version: 1.0.4 + resolution: "@metamask/solana-wallet-snap@npm:1.0.4" + checksum: 10/4d3b2a400607299fb4200e409f151e999e980e30521e63dcbbb5b658a4fdcfd6be341452695de32d985e14e8f8d4c5c24169f84dfd1a2063d3bc35f0c4903f74 languageName: node linkType: hard @@ -26614,7 +26614,7 @@ __metadata: "@metamask/snaps-rpc-methods": "npm:^11.7.0" "@metamask/snaps-sdk": "npm:^6.13.0" "@metamask/snaps-utils": "npm:^8.6.1" - "@metamask/solana-wallet-snap": "npm:^1.0.3" + "@metamask/solana-wallet-snap": "npm:^1.0.4" "@metamask/test-bundler": "npm:^1.0.0" "@metamask/test-dapp": "npm:8.13.0" "@metamask/transaction-controller": "npm:^42.0.0" From 13a1fcfc694f3f23b95e99510ad86258dfa381ff Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 15:07:00 +0000 Subject: [PATCH 6/7] feat: Display Unlimited for really large spending caps on Permit (#29102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Uses `UNLIMITED_THRESHOLD` to determine wether or not to show a permit amount as "Unlimited". Updates unit tests. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29102?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3763 ## **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/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../hooks/use-approve-token-simulation.ts | 5 +- .../confirm/info/shared/constants.ts | 2 + .../decoded-simulation.test.tsx | 31 +++++++- .../decoded-simulation/decoded-simulation.tsx | 5 ++ .../value-display/value-display.tsx | 74 +++++++++++-------- 5 files changed, 84 insertions(+), 33 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts index 74c857f7078b..b8605da19f54 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts @@ -8,12 +8,11 @@ import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions import { getIntlLocale } from '../../../../../../../ducks/locale/locale'; import { formatAmount } from '../../../../simulation-details/formatAmount'; import { useDecodedTransactionData } from '../../hooks/useDecodedTransactionData'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; import { useIsNFT } from './use-is-nft'; -const UNLIMITED_THRESHOLD = 10 ** 15; - function isSpendingCapUnlimited(decodedSpendingCap: number) { - return decodedSpendingCap >= UNLIMITED_THRESHOLD; + return decodedSpendingCap >= TOKEN_VALUE_UNLIMITED_THRESHOLD; } export const useApproveTokenSimulation = ( diff --git a/ui/pages/confirmations/components/confirm/info/shared/constants.ts b/ui/pages/confirmations/components/confirm/info/shared/constants.ts index 9f9b4bb7a2f1..9953383149d2 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/constants.ts +++ b/ui/pages/confirmations/components/confirm/info/shared/constants.ts @@ -1 +1,3 @@ export const HEX_ZERO = '0x0'; + +export const TOKEN_VALUE_UNLIMITED_THRESHOLD = 10 ** 15; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx index c06cf906e870..0cfd564d8a69 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx @@ -77,6 +77,35 @@ const decodingDataBidding: DecodingDataStateChanges = [ describe('DecodedSimulation', () => { it('renders component correctly', async () => { + const state = getMockTypedSignConfirmStateForRequest({ + ...permitSignatureMsg, + decodingLoading: false, + decodingData: { + ...decodingData, + stateChanges: decodingData.stateChanges + ? [ + { + ...decodingData.stateChanges[0], + amount: '12345', + }, + ] + : [], + }, + }); + + const mockStore = configureMockStore([])(state); + + const { findByText } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(await findByText('Estimated changes')).toBeInTheDocument(); + expect(await findByText('Spending cap')).toBeInTheDocument(); + expect(await findByText('12,345')).toBeInTheDocument(); + }); + + it('renders component correctly for a very large amount', async () => { const state = getMockTypedSignConfirmStateForRequest({ ...permitSignatureMsg, decodingLoading: false, @@ -91,7 +120,7 @@ describe('DecodedSimulation', () => { expect(await findByText('Estimated changes')).toBeInTheDocument(); expect(await findByText('Spending cap')).toBeInTheDocument(); - expect(await findByText('1,461,501,637,3...')).toBeInTheDocument(); + expect(await findByText('Unlimited')).toBeInTheDocument(); }); it('render correctly for ERC712 token', async () => { diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx index fe2be9d76261..9cfc0bdc1ae7 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx @@ -71,6 +71,10 @@ const StateChangeRow = ({ const { assetType, changeType, amount, contractAddress, tokenID } = stateChange; const tooltip = getStateChangeToolip(stateChangeList, stateChange, t); + const canDisplayValueAsUnlimited = + assetType === TokenStandard.ERC20 && + (changeType === DecodingDataChangeType.Approve || + changeType === DecodingDataChangeType.Revoke); return ( )} {assetType === 'NATIVE' && ( diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx index c41f99f2f320..1d59418d80c2 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx @@ -1,20 +1,11 @@ -import React, { useMemo } from 'react'; import { NameType } from '@metamask/name-controller'; import { Hex } from '@metamask/utils'; import { captureException } from '@sentry/browser'; - +import React, { useMemo } from 'react'; import { MetaMetricsEventLocation } from '../../../../../../../../../shared/constants/metametrics'; -import { shortenString } from '../../../../../../../../helpers/utils/util'; import { calcTokenAmount } from '../../../../../../../../../shared/lib/transactions-controller-utils'; import useTokenExchangeRate from '../../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate'; -import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display'; -import { - formatAmount, - formatAmountMaxPrecision, -} from '../../../../../simulation-details/formatAmount'; -import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails'; -import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation'; - +import Name from '../../../../../../../../components/app/name/name'; import { Box, Text, @@ -27,8 +18,17 @@ import { JustifyContent, TextAlign, } from '../../../../../../../../helpers/constants/design-system'; -import Name from '../../../../../../../../components/app/name/name'; +import { shortenString } from '../../../../../../../../helpers/utils/util'; +import { useI18nContext } from '../../../../../../../../hooks/useI18nContext'; +import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails'; +import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation'; import { TokenDetailsERC20 } from '../../../../../../utils/token'; +import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display'; +import { + formatAmount, + formatAmountMaxPrecision, +} from '../../../../../simulation-details/formatAmount'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../../shared/constants'; import { getAmountColors } from '../../../utils'; type PermitSimulationValueDisplayParams = { @@ -56,6 +56,9 @@ type PermitSimulationValueDisplayParams = { /** True if value is being debited to wallet */ debit?: boolean; + + /** Whether a large amount can be substituted by "Unlimited" */ + canDisplayValueAsUnlimited?: boolean; }; const PermitSimulationValueDisplay: React.FC< @@ -68,7 +71,10 @@ const PermitSimulationValueDisplay: React.FC< value, credit, debit, + canDisplayValueAsUnlimited, }) => { + const t = useI18nContext(); + const exchangeRate = useTokenExchangeRate(tokenContract); const tokenDetails = useGetTokenStandardAndDetails(tokenContract); @@ -88,18 +94,26 @@ const PermitSimulationValueDisplay: React.FC< return undefined; }, [exchangeRate, tokenDecimals, value]); - const { tokenValue, tokenValueMaxPrecision } = useMemo(() => { - if (!value || tokenId) { - return { tokenValue: null, tokenValueMaxPrecision: null }; - } + const { tokenValue, tokenValueMaxPrecision, shouldShowUnlimitedValue } = + useMemo(() => { + if (!value || tokenId) { + return { + tokenValue: null, + tokenValueMaxPrecision: null, + shouldShowUnlimitedValue: false, + }; + } - const tokenAmount = calcTokenAmount(value, tokenDecimals); + const tokenAmount = calcTokenAmount(value, tokenDecimals); - return { - tokenValue: formatAmount('en-US', tokenAmount), - tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount), - }; - }, [tokenDecimals, value]); + return { + tokenValue: formatAmount('en-US', tokenAmount), + tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount), + shouldShowUnlimitedValue: + canDisplayValueAsUnlimited && + Number(value) > TOKEN_VALUE_UNLIMITED_THRESHOLD, + }; + }, [tokenDecimals, value]); /** Temporary error capturing as we are building out Permit Simulations */ if (!tokenContract) { @@ -138,13 +152,15 @@ const PermitSimulationValueDisplay: React.FC< > {credit && '+ '} {debit && '- '} - {tokenValue !== null && - shortenString(tokenValue || '', { - truncatedCharLimit: 15, - truncatedStartChars: 15, - truncatedEndChars: 0, - skipCharacterInEnd: true, - })} + {shouldShowUnlimitedValue + ? t('unlimited') + : tokenValue !== null && + shortenString(tokenValue || '', { + truncatedCharLimit: 15, + truncatedStartChars: 15, + truncatedEndChars: 0, + skipCharacterInEnd: true, + })} {tokenId && `#${tokenId}`} From 22490c38d46f993633d35f5f1f56224894a929ba Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 15:19:21 +0000 Subject: [PATCH 7/7] fix: Wrong icon for ETH on L2s being displayed on transfer confirmations (#29353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** We were inadvertently referencing the network icon map instead of the native token map. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29353?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29351 ## **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/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../__snapshots__/native-transfer.test.tsx.snap | 2 +- .../native-send-heading/native-send-heading.tsx | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap index 4fe5c3f41bbd..406fd66ae962 100644 --- a/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap @@ -8,7 +8,7 @@ exports[`NativeTransferInfo renders correctly 1`] = `
- G + E

{ const multichainNetwork = useSelector(getMultichainNetwork); const ticker = multichainNetwork?.network?.ticker; + const networkConfigurationsByChainId = useSelector( + getNetworkConfigurationsByChainId, + ); + + const network = networkConfigurationsByChainId?.[transactionMeta.chainId]; + const { nativeCurrency } = network; + const locale = useSelector(getIntlLocale); const roundedTransferValue = formatAmount(locale, nativeAssetTransferValue); @@ -76,12 +84,11 @@ const NativeSendHeading = () => { const NetworkImage = (