From 40f721c34cd99e60d3439c9c24ca585ffba971f5 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Thu, 11 Dec 2025 14:48:15 +0100 Subject: [PATCH 1/7] fix: added virtualization to nfts list --- .../app/assets/nfts/nft-grid/nft-grid.tsx | 97 ++++++++++++++++--- 1 file changed, 84 insertions(+), 13 deletions(-) diff --git a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx index ba8bfccb356b..6645e2256623 100644 --- a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx +++ b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx @@ -1,6 +1,7 @@ -import React from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; import { toHex } from '@metamask/controller-utils'; +import { useVirtualizer } from '@tanstack/react-virtual'; import { AlignItems, Display, @@ -18,6 +19,7 @@ import useGetAssetImageUrl from '../../../../../hooks/useGetAssetImageUrl'; import { getImageForChainId } from '../../../../../selectors/multichain'; import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks'; import useFetchNftDetailsFromTokenURI from '../../../../../hooks/useFetchNftDetailsFromTokenURI'; +import { useScrollContainer } from '../../../../../contexts/scroll-container'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths import { isWebUrl } from '../../../../../../app/scripts/lib/util'; @@ -71,6 +73,9 @@ const NFTGridItem = (props: { ); }; +// Breakpoint matches design-system $screen-md-max (768px - 1px) +const SCREEN_MD_MAX = 767; + // TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860 // eslint-disable-next-line @typescript-eslint/naming-convention export default function NftGrid({ @@ -82,30 +87,96 @@ export default function NftGrid({ handleNftClick: (nft: NFT) => void; privacyMode?: boolean; }) { + const scrollContainerRef = useScrollContainer(); const nftsStillFetchingIndication = useSelector( getNftIsStillFetchingIndication, ); + // Detect screen size to match CSS Grid column count + // 4 columns for large screens, 3 columns for medium and below + const [itemsPerRow, setItemsPerRow] = useState(() => + window.innerWidth > SCREEN_MD_MAX ? 4 : 3, + ); + + useEffect(() => { + const mediaQuery = window.matchMedia(`(max-width: ${SCREEN_MD_MAX}px)`); + + const handleResize = (e: MediaQueryListEvent) => { + setItemsPerRow(e.matches ? 3 : 4); + }; + + mediaQuery.addEventListener('change', handleResize); + return () => mediaQuery.removeEventListener('change', handleResize); + }, []); + + // Group NFTs into rows for virtualization + const nftRows = useMemo(() => { + const rows: NFT[][] = []; + for (let i = 0; i < nfts.length; i += itemsPerRow) { + rows.push(nfts.slice(i, i + itemsPerRow)); + } + return rows; + }, [nfts, itemsPerRow]); + + const virtualizer = useVirtualizer({ + count: nftRows.length, + getScrollElement: () => scrollContainerRef?.current || null, + estimateSize: () => 200, + overscan: 5, + }); + return ( - - {nfts.map((nft: NFT, index: number) => { +
+ {virtualizer.getVirtualItems().map((virtualRow) => { + const rowNfts = nftRows[virtualRow.index]; return ( - null}> +
- handleNftClick(nft)} - privacyMode={privacyMode} - /> + {rowNfts.map((nft, index) => ( + null} + > + + handleNftClick(nft)} + privacyMode={privacyMode} + /> + + + ))} - +
); })} - +
{nftsStillFetchingIndication ? ( Date: Fri, 12 Dec 2025 10:24:47 +0100 Subject: [PATCH 2/7] chore: updated tests --- .../nft-grid/__snapshots__/nft-grid.test.tsx.snap | 3 ++- .../app/assets/nfts/nft-grid/nft-grid.test.tsx | 15 +++++++++++++++ .../app/assets/nfts/nft-grid/nft-grid.tsx | 8 ++------ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap b/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap index eb60e040dacc..e3bbcc279c93 100644 --- a/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap +++ b/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap @@ -7,7 +7,8 @@ exports[`NftGrid matches the snapshot 1`] = ` style="margin: 16px;" >
diff --git a/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx b/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx index e465f327a9e9..a2d70bf38a67 100644 --- a/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx +++ b/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx @@ -17,6 +17,21 @@ jest.mock('../../../../../selectors', () => ({ getNftIsStillFetchingIndication: jest.fn(), })); +// Mock window.matchMedia +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation((query) => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // Deprecated + removeListener: jest.fn(), // Deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +}); + describe('NftGrid', () => { const mockStore = configureStore([]); diff --git a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx index 6645e2256623..97308c794b5d 100644 --- a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx +++ b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx @@ -128,10 +128,9 @@ export default function NftGrid({ return (
{virtualizer.getVirtualItems().map((virtualRow) => { @@ -141,11 +140,8 @@ export default function NftGrid({ key={virtualRow.index} data-index={virtualRow.index} ref={virtualizer.measureElement} + className="absolute top-0 left-0 w-full" style={{ - position: 'absolute', - top: 0, - left: 0, - width: '100%', transform: `translateY(${virtualRow.start}px)`, paddingBottom: '16px', }} From a4099ab51e2831ac5689c41f3224cf9e2903ffc1 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 12 Dec 2025 11:20:32 +0100 Subject: [PATCH 3/7] fix: broken tests --- .../asset-picker-modal.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx index ba4f76ab13f8..6538829fd801 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx @@ -110,6 +110,21 @@ jest.mock( }), ); +// Mock window.matchMedia for all tests +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation((query) => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // Deprecated + removeListener: jest.fn(), // Deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +}); + describe('AssetPickerModal', () => { const useSelectorMock = useSelector as jest.Mock; const useI18nContextMock = useI18nContext as jest.Mock; From 8a1ab2571d90f72ce73ad60f2ff663d4d8ff8f77 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 12 Dec 2025 13:47:57 +0100 Subject: [PATCH 4/7] fix: broken tests --- test/jest/setup.js | 15 +++++++++++++++ .../app/assets/nfts/nft-grid/nft-grid.test.tsx | 15 --------------- .../asset-picker-modal.test.tsx | 15 --------------- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/test/jest/setup.js b/test/jest/setup.js index c73c088a6091..02b6283d72e1 100644 --- a/test/jest/setup.js +++ b/test/jest/setup.js @@ -9,6 +9,21 @@ jest.mock('webextension-polyfill', () => { }; }); +// Mock window.matchMedia +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation((query) => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // Deprecated + removeListener: jest.fn(), // Deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +}); + const UNRESOLVED = Symbol('timedOut'); // Store this in case it gets stubbed later diff --git a/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx b/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx index a2d70bf38a67..e465f327a9e9 100644 --- a/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx +++ b/ui/components/app/assets/nfts/nft-grid/nft-grid.test.tsx @@ -17,21 +17,6 @@ jest.mock('../../../../../selectors', () => ({ getNftIsStillFetchingIndication: jest.fn(), })); -// Mock window.matchMedia -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation((query) => ({ - matches: false, - media: query, - onchange: null, - addListener: jest.fn(), // Deprecated - removeListener: jest.fn(), // Deprecated - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), -}); - describe('NftGrid', () => { const mockStore = configureStore([]); diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx index 6538829fd801..ba4f76ab13f8 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.test.tsx @@ -110,21 +110,6 @@ jest.mock( }), ); -// Mock window.matchMedia for all tests -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation((query) => ({ - matches: false, - media: query, - onchange: null, - addListener: jest.fn(), // Deprecated - removeListener: jest.fn(), // Deprecated - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), -}); - describe('AssetPickerModal', () => { const useSelectorMock = useSelector as jest.Mock; const useI18nContextMock = useI18nContext as jest.Mock; From b18e65700c86d6b6d9c4bd1e47793270d3dc149d Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 12 Dec 2025 14:39:39 +0100 Subject: [PATCH 5/7] fix: broken tests --- test/helpers/setup-helper.js | 15 ++++++++++++++- test/jest/setup.js | 15 --------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/test/helpers/setup-helper.js b/test/helpers/setup-helper.js index 7bb8345bd1eb..fc1563e31521 100644 --- a/test/helpers/setup-helper.js +++ b/test/helpers/setup-helper.js @@ -134,7 +134,20 @@ window.localStorage = { }; // used for native dark/light mode detection -window.matchMedia = () => true; +window.matchMedia = () => ({ + matches: false, + media: '', + onchange: null, + // eslint-disable-next-line no-empty-function + addListener: () => {}, // Deprecated + // eslint-disable-next-line no-empty-function + removeListener: () => {}, // Deprecated + // eslint-disable-next-line no-empty-function + addEventListener: () => {}, + // eslint-disable-next-line no-empty-function + removeEventListener: () => {}, + dispatchEvent: () => true, +}); // override @metamask/logo window.requestAnimationFrame = () => undefined; diff --git a/test/jest/setup.js b/test/jest/setup.js index 02b6283d72e1..c73c088a6091 100644 --- a/test/jest/setup.js +++ b/test/jest/setup.js @@ -9,21 +9,6 @@ jest.mock('webextension-polyfill', () => { }; }); -// Mock window.matchMedia -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation((query) => ({ - matches: false, - media: query, - onchange: null, - addListener: jest.fn(), // Deprecated - removeListener: jest.fn(), // Deprecated - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), -}); - const UNRESOLVED = Symbol('timedOut'); // Store this in case it gets stubbed later From 2081b66787b87ddd9dfcea02eefbb5ff7c3adb48 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 12 Dec 2025 15:33:09 +0100 Subject: [PATCH 6/7] chore: fix broken tests --- .../asset-picker-modal-nft-tab.test.tsx | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-nft-tab.test.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-nft-tab.test.tsx index 8c10b4ee13f4..3be544b19ce9 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-nft-tab.test.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal-nft-tab.test.tsx @@ -6,6 +6,7 @@ import nock from 'nock'; import { renderWithProvider } from '../../../../../test/lib/render-helpers-navigate'; import mockState from '../../../../../test/data/mock-state.json'; import { mockNetworkState } from '../../../../../test/stub/networks'; +import { ScrollContainer } from '../../../../contexts/scroll-container'; import { AssetPickerModalNftTab } from './asset-picker-modal-nft-tab'; jest.mock('../../../../hooks/useGetAssetImageUrl', () => ({ @@ -15,6 +16,37 @@ jest.mock('../../../../hooks/useGetAssetImageUrl', () => ({ default: () => 'mock-image-url.png', })); +// Mock element measurements for virtualization +Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 600, +}); + +Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + value: 400, +}); + +// Mock getBoundingClientRect for virtualizer +HTMLElement.prototype.getBoundingClientRect = jest.fn(() => ({ + width: 400, + height: 600, + top: 0, + left: 0, + bottom: 600, + right: 400, + x: 0, + y: 0, + toJSON: () => ({}), +})); + +// Mock ResizeObserver for virtualizer +global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), +})); + const defaultProps = { searchQuery: '', onClose: jest.fn(), @@ -98,7 +130,9 @@ describe('AssetPickerModalNftTab', () => { }, }); const { getByText, getAllByTestId } = renderWithProvider( - , + + + , mainnetStore, ); @@ -126,7 +160,9 @@ describe('AssetPickerModalNftTab', () => { }); const { getAllByTestId } = renderWithProvider( - , + + + , lineaStore, ); From da8b6455e34c3b2fa17b349baf47a222b20af8ff Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 15 Dec 2025 11:10:29 +0100 Subject: [PATCH 7/7] fix: broken tests --- test/helpers/setup-helper.js | 11 +- .../__snapshots__/nft-grid.test.tsx.snap | 3 +- .../app/assets/nfts/nft-grid/nft-grid.tsx | 114 +++++++++++------- .../asset-picker-modal-nft-tab.test.tsx | 40 +----- 4 files changed, 74 insertions(+), 94 deletions(-) diff --git a/test/helpers/setup-helper.js b/test/helpers/setup-helper.js index fc1563e31521..c9a29cebe4ab 100644 --- a/test/helpers/setup-helper.js +++ b/test/helpers/setup-helper.js @@ -135,18 +135,9 @@ window.localStorage = { // used for native dark/light mode detection window.matchMedia = () => ({ - matches: false, - media: '', - onchange: null, - // eslint-disable-next-line no-empty-function - addListener: () => {}, // Deprecated - // eslint-disable-next-line no-empty-function - removeListener: () => {}, // Deprecated + // Used for NFT list virtualization // eslint-disable-next-line no-empty-function addEventListener: () => {}, - // eslint-disable-next-line no-empty-function - removeEventListener: () => {}, - dispatchEvent: () => true, }); // override @metamask/logo diff --git a/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap b/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap index e3bbcc279c93..eb60e040dacc 100644 --- a/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap +++ b/ui/components/app/assets/nfts/nft-grid/__snapshots__/nft-grid.test.tsx.snap @@ -7,8 +7,7 @@ exports[`NftGrid matches the snapshot 1`] = ` style="margin: 16px;" >
diff --git a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx index 97308c794b5d..1f31b0bba87a 100644 --- a/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx +++ b/ui/components/app/assets/nfts/nft-grid/nft-grid.tsx @@ -125,54 +125,80 @@ export default function NftGrid({ overscan: 5, }); + // Disable virtualization when scroll container is not available (e.g., in tests) + // or when there are few NFTs (no performance benefit) + const shouldDisableVirtualization = + !scrollContainerRef?.current || nfts.length <= 20; + return ( -
- {virtualizer.getVirtualItems().map((virtualRow) => { - const rowNfts = nftRows[virtualRow.index]; - return ( -
- + {nfts.map((nft: NFT, index: number) => { + return ( + null}> + + handleNftClick(nft)} + privacyMode={privacyMode} + /> + + + ); + })} + + ) : ( +
+ {virtualizer.getVirtualItems().map((virtualRow) => { + const rowNfts = nftRows[virtualRow.index]; + return ( +
- {rowNfts.map((nft, index) => ( - null} - > - + {rowNfts.map((nft, index) => ( + null} > - handleNftClick(nft)} - privacyMode={privacyMode} - /> - - - ))} - -
- ); - })} -
+ + handleNftClick(nft)} + privacyMode={privacyMode} + /> + + + ))} + +
+ ); + })} +
+ )} {nftsStillFetchingIndication ? ( ({ @@ -16,37 +15,6 @@ jest.mock('../../../../hooks/useGetAssetImageUrl', () => ({ default: () => 'mock-image-url.png', })); -// Mock element measurements for virtualization -Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { - configurable: true, - value: 600, -}); - -Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { - configurable: true, - value: 400, -}); - -// Mock getBoundingClientRect for virtualizer -HTMLElement.prototype.getBoundingClientRect = jest.fn(() => ({ - width: 400, - height: 600, - top: 0, - left: 0, - bottom: 600, - right: 400, - x: 0, - y: 0, - toJSON: () => ({}), -})); - -// Mock ResizeObserver for virtualizer -global.ResizeObserver = jest.fn().mockImplementation(() => ({ - observe: jest.fn(), - unobserve: jest.fn(), - disconnect: jest.fn(), -})); - const defaultProps = { searchQuery: '', onClose: jest.fn(), @@ -130,9 +98,7 @@ describe('AssetPickerModalNftTab', () => { }, }); const { getByText, getAllByTestId } = renderWithProvider( - - - , + , mainnetStore, ); @@ -160,9 +126,7 @@ describe('AssetPickerModalNftTab', () => { }); const { getAllByTestId } = renderWithProvider( - - - , + , lineaStore, );