diff --git a/app/components/UI/CollectibleContracts/index.js b/app/components/UI/CollectibleContracts/index.js index 598a60a79e71..76201836d13d 100644 --- a/app/components/UI/CollectibleContracts/index.js +++ b/app/components/UI/CollectibleContracts/index.js @@ -12,7 +12,7 @@ import { RefreshControl, ActivityIndicator, } from 'react-native'; -import { useTailwind } from '@metamask/design-system-twrnc-preset'; + import { FlashList } from '@shopify/flash-list'; import { connect, useSelector } from 'react-redux'; import { fontStyles } from '../../../styles/common'; @@ -176,7 +176,6 @@ const CollectibleContracts = ({ }) => { // Start tracing component loading const isFirstRender = useRef(true); - const tw = useTailwind(); if (isFirstRender.current) { trace({ name: TraceName.CollectibleContractsComponent }); } @@ -500,16 +499,21 @@ const CollectibleContracts = ({ const renderEmpty = useCallback( () => ( - + <> + {!isNftFetchingProgress && ( + + )} + ), - [goToAddCollectible, tw, isAddNFTEnabled], + [goToAddCollectible, isAddNFTEnabled, isNftFetchingProgress], ); const renderList = useCallback( @@ -564,6 +568,17 @@ const CollectibleContracts = ({ chainId: firstEnabledChainId, }); + // Determine if we should show the network selector + // Show when there are NFTs (contracts, collectibles, or favorites) + // Memoized to avoid unnecessary recalculations on every render + const shouldShowNetworkSelector = useMemo( + () => + filteredCollectibleContracts.length > 0 || + collectibles.length > 0 || + favoriteCollectibles.length > 0, + [filteredCollectibleContracts, collectibles, favoriteCollectibles], + ); + // End trace when component has finished initial loading useEffect(() => { endTrace({ name: TraceName.CollectibleContractsComponent }); @@ -575,60 +590,62 @@ const CollectibleContracts = ({ style={styles.BarWrapper} testID={WalletViewSelectorsIDs.NFT_TAB_CONTAINER} > - - - - {isRemoveGlobalNetworkSelectorEnabled() ? ( - - {!areAllNetworksSelected && ( - - )} + {shouldShowNetworkSelector && ( + + + + {isRemoveGlobalNetworkSelectorEnabled() ? ( + + {!areAllNetworksSelected && ( + + )} + + {enabledNetworks.length > 1 + ? strings('wallet.popular_networks') + : currentNetworkName ?? + strings('wallet.current_network')} + + + ) : ( - {enabledNetworks.length > 1 + {isAllNetworks && isPopularNetwork && isEvmSelected ? strings('wallet.popular_networks') - : currentNetworkName ?? - strings('wallet.current_network')} + : networkName ?? strings('wallet.current_network')} - - ) : ( - - {isAllNetworks && isPopularNetwork && isEvmSelected - ? strings('wallet.popular_networks') - : networkName ?? strings('wallet.current_network')} - - )} - - } - isDisabled={isDisabled} - onPress={showFilterControls} - endIconName={ - isEvmSelected || isMultichainAccountsState2Enabled - ? IconName.ArrowDown - : undefined - } - style={ - isDisabled ? styles.controlButtonDisabled : styles.controlButton - } - disabled={isDisabled} - /> + )} + + } + isDisabled={isDisabled} + onPress={showFilterControls} + endIconName={ + isEvmSelected || isMultichainAccountsState2Enabled + ? IconName.ArrowDown + : undefined + } + style={ + isDisabled ? styles.controlButtonDisabled : styles.controlButton + } + disabled={isDisabled} + /> + - + )} {renderList()} ); diff --git a/app/components/UI/CollectibleContracts/index.test.tsx b/app/components/UI/CollectibleContracts/index.test.tsx index 4decc125f042..5e2ed1f9d090 100644 --- a/app/components/UI/CollectibleContracts/index.test.tsx +++ b/app/components/UI/CollectibleContracts/index.test.tsx @@ -1103,8 +1103,9 @@ describe('CollectibleContracts', () => { }); }); - it('shows filter controls when evm is selected', () => { - const mockState: DeepPartial = { + it('hides network selector when NFTs are empty', () => { + // Test with completely empty NFT state + const emptyState: DeepPartial = { collectibles: { favorites: {}, }, @@ -1119,13 +1120,52 @@ describe('CollectibleContracts', () => { ticker: 'ETH', }), }, - AccountTrackerController: { - accountsByChainId: { - '0x1': { - [MOCK_ADDRESS]: { balance: '0' }, - }, + AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, + PreferencesController: { + displayNftMedia: false, + isIpfsGatewayEnabled: false, + tokenNetworkFilter: { + '0x1': true, }, + } as unknown as PreferencesState, + NftController: { + allNfts: {}, + allNftContracts: {}, }, + }, + }, + }; + + const { queryByTestId } = renderWithProvider(, { + state: emptyState, + }); + + // Network selector should NOT be present in empty state + expect(queryByTestId('collectibles-network-filter')).toBeNull(); + + // Should show empty state instead + expect(queryByTestId('collectibles-empty-state')).toBeDefined(); + }); + + it('tests conditional logic by verifying empty state is shown when no NFTs', () => { + // This test verifies that when there are no NFTs, we show the empty state + // instead of the network selector, which confirms our conditional logic + const emptyState: DeepPartial = { + collectibles: { + favorites: {}, + }, + engine: { + backgroundState: { + ...backgroundState, + NetworkController: { + ...mockNetworkState({ + chainId: CHAIN_IDS.MAINNET, + id: 'mainnet', + nickname: 'Ethereum Mainnet', + ticker: 'ETH', + }), + }, + AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, PreferencesController: { displayNftMedia: false, isIpfsGatewayEnabled: false, @@ -1133,44 +1173,34 @@ describe('CollectibleContracts', () => { '0x1': true, }, } as unknown as PreferencesState, - AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, NftController: { - allNfts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, - allNftContracts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, + allNfts: {}, + allNftContracts: {}, }, }, }, }; - const mockNavigation = { - navigate: jest.fn(), - push: jest.fn(), - }; - const { getByTestId } = renderWithProvider( - , - { - state: mockState, - }, - ); - const filterControlersButton = getByTestId('collectibles-network-filter'); - fireEvent.press(filterControlersButton); + const { queryByTestId } = renderWithProvider(, { + state: emptyState, + }); + + // When no NFTs exist: + // 1. Network selector should be hidden + expect(queryByTestId('collectibles-network-filter')).toBeNull(); - expect(mockNavigation.navigate).toHaveBeenCalledTimes(1); + // 2. Empty state should be shown + expect(queryByTestId('collectibles-empty-state')).toBeDefined(); + + // This confirms our conditional rendering logic is working correctly }); - it('shows network manager when isRemoveGlobalNetworkSelectorEnabled is true', () => { + it('verifies conditional rendering with network manager settings', () => { const networksModule = jest.requireMock('../../../util/networks'); networksModule.isRemoveGlobalNetworkSelectorEnabled.mockReturnValue(true); - const mockState: DeepPartial = { + // Test that network selector is hidden regardless of network manager settings when no NFTs + const emptyState: DeepPartial = { collectibles: { favorites: {}, }, @@ -1185,13 +1215,7 @@ describe('CollectibleContracts', () => { ticker: 'ETH', }), }, - AccountTrackerController: { - accountsByChainId: { - '0x1': { - [MOCK_ADDRESS]: { balance: '0' }, - }, - }, - }, + AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, PreferencesController: { displayNftMedia: false, isIpfsGatewayEnabled: false, @@ -1199,42 +1223,23 @@ describe('CollectibleContracts', () => { '0x1': true, }, } as unknown as PreferencesState, - AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, NftController: { - allNfts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, - allNftContracts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, + allNfts: {}, + allNftContracts: {}, }, }, }, }; - const mockNavigation = { - navigate: jest.fn(), - push: jest.fn(), - }; - const { getByTestId } = renderWithProvider( - , - { - state: mockState, - }, - ); - const filterControlersButton = getByTestId('collectibles-network-filter'); - fireEvent.press(filterControlersButton); + const { queryByTestId } = renderWithProvider(, { + state: emptyState, + }); - expect(mockNavigation.navigate).toHaveBeenCalledWith( - 'RootModalFlow', - expect.objectContaining({ - screen: 'NetworkManager', - }), - ); + // Network selector should be hidden when no NFTs, regardless of network manager config + expect(queryByTestId('collectibles-network-filter')).toBeNull(); + + // Should show empty state with import button instead + expect(queryByTestId('collectibles-empty-state')).toBeDefined(); }); it('filters collectibles by enabled networks when isRemoveGlobalNetworkSelectorEnabled is true', () => { @@ -1330,11 +1335,12 @@ describe('CollectibleContracts', () => { expect(nftImage.props.source.uri).toEqual(nftItemData[0].image); }); - it('shows enabled networks text when isRemoveGlobalNetworkSelectorEnabled is true and multiple networks enabled', () => { + it('verifies conditional rendering with multiple networks enabled', () => { const networksModule = jest.requireMock('../../../util/networks'); networksModule.isRemoveGlobalNetworkSelectorEnabled.mockReturnValue(true); - const mockState: DeepPartial = { + // Test that network selector is hidden when no NFTs exist, even with multiple networks + const emptyState: DeepPartial = { collectibles: { favorites: {}, }, @@ -1349,42 +1355,35 @@ describe('CollectibleContracts', () => { ticker: 'ETH', }), }, - AccountTrackerController: { - accountsByChainId: { - '0x1': { - [MOCK_ADDRESS]: { balance: '0' }, - }, - }, - }, + AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, PreferencesController: { displayNftMedia: false, isIpfsGatewayEnabled: false, tokenNetworkFilter: { '0x1': true, - '0x89': true, // Polygon network enabled + '0x89': true, // Multiple networks enabled }, } as unknown as PreferencesState, - AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE, NftController: { - allNfts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, - allNftContracts: { - [MOCK_ADDRESS]: { - '0x1': [], - }, - }, + allNfts: {}, + allNftContracts: {}, }, }, }, }; - const { getByText } = renderWithProvider(, { - state: mockState, - }); + const { queryByTestId, queryByText } = renderWithProvider( + , + { + state: emptyState, + }, + ); + + // Network selector should be hidden when no NFTs, regardless of multiple networks + expect(queryByTestId('collectibles-network-filter')).toBeNull(); + expect(queryByText('Popular networks')).toBeNull(); - expect(getByText('Popular networks')).toBeDefined(); + // Should show empty state instead + expect(queryByTestId('collectibles-empty-state')).toBeDefined(); }); }); diff --git a/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.test.tsx b/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.test.tsx index 8ac2b905a496..8601e4fa50c4 100644 --- a/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.test.tsx +++ b/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.test.tsx @@ -24,15 +24,13 @@ describe('CollectiblesEmptyState', () => { // Button should not render when no onAction is provided }); - it('calls onDiscoverCollectibles when action button is pressed', () => { - const mockOnDiscoverCollectibles = jest.fn(); + it('calls onAction when action button is pressed', () => { + const mockOnAction = jest.fn(); const { getByText } = renderWithProvider( - , + , ); fireEvent.press(getByText('Import NFTs')); - expect(mockOnDiscoverCollectibles).toHaveBeenCalledTimes(1); + expect(mockOnAction).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.tsx b/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.tsx index 86f5c8ec980f..43eaec9f46d9 100644 --- a/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.tsx +++ b/app/components/UI/CollectiblesEmptyState/CollectiblesEmptyState.tsx @@ -11,11 +11,11 @@ import emptyStateNftsLight from '../../../images/empty-state-nfts-light.png'; import emptyStateNftsDark from '../../../images/empty-state-nfts-dark.png'; interface CollectiblesEmptyStateProps extends TabEmptyStateProps { - onDiscoverCollectibles?: () => void; + onAction?: () => void; } export const CollectiblesEmptyState: React.FC = ({ - onDiscoverCollectibles, + onAction, ...props }) => { const collectiblesImage = useAssetFromTheme( @@ -34,7 +34,7 @@ export const CollectiblesEmptyState: React.FC = ({ } description={strings('wallet.nft_empty_description')} actionButtonText={strings('wallet.discover_nfts')} - onAction={onDiscoverCollectibles} + onAction={onAction} {...props} /> ); diff --git a/app/components/UI/DeFiPositions/DeFiPositionsList.test.tsx b/app/components/UI/DeFiPositions/DeFiPositionsList.test.tsx index 748dd18967e6..f6deaaeefa62 100644 --- a/app/components/UI/DeFiPositions/DeFiPositionsList.test.tsx +++ b/app/components/UI/DeFiPositions/DeFiPositionsList.test.tsx @@ -328,9 +328,6 @@ describe('DeFiPositionsList', () => { expect( await findByTestId(WalletViewSelectorsIDs.DEFI_POSITIONS_CONTAINER), ).toBeOnTheScreen(); - expect( - await findByTestId(WalletViewSelectorsIDs.DEFI_POSITIONS_NETWORK_FILTER), - ).toBeOnTheScreen(); expect( await findByText(`Lend, borrow, and trade, right in your wallet.`), ).toBeOnTheScreen(); @@ -446,11 +443,6 @@ describe('DeFiPositionsList', () => { expect( await findByTestId(WalletViewSelectorsIDs.DEFI_POSITIONS_CONTAINER), ).toBeOnTheScreen(); - expect( - await findByTestId( - WalletViewSelectorsIDs.DEFI_POSITIONS_NETWORK_FILTER, - ), - ).toBeOnTheScreen(); expect( await findByText(`Lend, borrow, and trade, right in your wallet.`), ).toBeOnTheScreen(); diff --git a/app/components/UI/DeFiPositions/DeFiPositionsList.tsx b/app/components/UI/DeFiPositions/DeFiPositionsList.tsx index cb49048c6684..f80adda51a72 100644 --- a/app/components/UI/DeFiPositions/DeFiPositionsList.tsx +++ b/app/components/UI/DeFiPositions/DeFiPositionsList.tsx @@ -2,7 +2,6 @@ import React, { useMemo } from 'react'; import { View, FlatList } from 'react-native'; import { strings } from '../../../../locales/i18n'; import { useSelector } from 'react-redux'; -import { useTailwind } from '@metamask/design-system-twrnc-preset'; import { selectChainId, selectIsAllNetworks, @@ -41,7 +40,6 @@ export interface DeFiPositionsListProps { const DeFiPositionsList: React.FC = () => { const { styles } = useStyles(styleSheet, undefined); - const tw = useTailwind(); const isAllNetworks = useSelector(selectIsAllNetworks); const currentChainId = useSelector(selectChainId) as Hex; const tokenSortConfig = useSelector(selectTokenSortConfig); @@ -138,8 +136,7 @@ const DeFiPositionsList: React.FC = () => { // No positions found for the current account return ( - - + ); } diff --git a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.styles.ts b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.styles.ts index 135653ca3c73..09cfb82a1b9d 100644 --- a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.styles.ts +++ b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.styles.ts @@ -9,12 +9,6 @@ const styleSheet = (params: { theme: Theme }) => { tradeInfoContainer: { paddingBottom: 12, }, - firstTimeIcon: { - width: 48, - height: 48, - marginTop: 16, - marginBottom: 8, - }, wrapper: { flex: 1, backgroundColor: colors.background.default, @@ -25,7 +19,6 @@ const styleSheet = (params: { theme: Theme }) => { contentContainer: { flexGrow: 1, }, - section: {}, sectionHeader: { flexDirection: 'row', justifyContent: 'space-between', diff --git a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx index f44fdc8b5acb..5704be34ea4a 100644 --- a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx +++ b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.test.tsx @@ -535,7 +535,7 @@ describe('PerpsTabView', () => { it('should have pull-to-refresh functionality configured', async () => { const mockLoadPositions = jest.fn(); mockUsePerpsLivePositions.mockReturnValue({ - positions: [], + positions: [mockPosition], // Add positions so control bar renders isLoading: false, isRefreshing: false, loadPositions: mockLoadPositions, @@ -567,6 +567,14 @@ describe('PerpsTabView', () => { return undefined; }); + // Add positions so control bar renders + mockUsePerpsLivePositions.mockReturnValue({ + positions: [mockPosition], + isLoading: false, + isRefreshing: false, + loadPositions: jest.fn(), + }); + render(); const manageBalanceButton = screen.getByTestId('manage-balance-button'); @@ -602,6 +610,14 @@ describe('PerpsTabView', () => { return undefined; }); + // Add positions so control bar renders + mockUsePerpsLivePositions.mockReturnValue({ + positions: [mockPosition], + isLoading: false, + isRefreshing: false, + loadPositions: jest.fn(), + }); + render(); const manageBalanceButton = screen.getByTestId('manage-balance-button'); @@ -633,6 +649,14 @@ describe('PerpsTabView', () => { return undefined; }); + // Add positions so control bar renders + mockUsePerpsLivePositions.mockReturnValue({ + positions: [mockPosition], + isLoading: false, + isRefreshing: false, + loadPositions: jest.fn(), + }); + render(); const manageBalanceButton = screen.getByTestId('manage-balance-button'); @@ -674,8 +698,11 @@ describe('PerpsTabView', () => { render(); // Assert - Component should render empty state with correct testID - expect(screen.getByTestId('manage-balance-button')).toBeOnTheScreen(); expect(screen.getByTestId('perps-empty-state')).toBeOnTheScreen(); + // Control bar should not be rendered in empty state + expect( + screen.queryByTestId('manage-balance-button'), + ).not.toBeOnTheScreen(); }); it('should pass correct hasPositions prop to PerpsTabControlBar when positions exist', () => { @@ -708,7 +735,7 @@ describe('PerpsTabView', () => { expect(screen.getByTestId('has-orders')).toHaveTextContent('true'); }); - it('should pass false for both props when no positions or orders exist', () => { + it('should not render control bar when no positions or orders exist', () => { mockUsePerpsLivePositions.mockReturnValue({ positions: [], isInitialLoading: false, @@ -718,13 +745,27 @@ describe('PerpsTabView', () => { render(); - expect(screen.getByTestId('has-positions')).toHaveTextContent('false'); - expect(screen.getByTestId('has-orders')).toHaveTextContent('false'); + // Control bar should not be rendered in empty state + expect(screen.queryByTestId('has-positions')).not.toBeOnTheScreen(); + expect(screen.queryByTestId('has-orders')).not.toBeOnTheScreen(); + expect( + screen.queryByTestId('manage-balance-button'), + ).not.toBeOnTheScreen(); + // Should show empty state instead + expect(screen.getByTestId('perps-empty-state')).toBeOnTheScreen(); }); }); describe('Accessibility', () => { it('should have proper accessibility for manage balance button', () => { + // Add positions so control bar renders + mockUsePerpsLivePositions.mockReturnValue({ + positions: [mockPosition], + isLoading: false, + isRefreshing: false, + loadPositions: jest.fn(), + }); + render(); const manageBalanceButton = screen.getByTestId('manage-balance-button'); diff --git a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx index 61d397e4bd57..a44b1f41b269 100644 --- a/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx +++ b/app/components/UI/Perps/Views/PerpsTabView/PerpsTabView.tsx @@ -239,27 +239,30 @@ const PerpsTabView: React.FC = () => { return ( <> - - - - {!isInitialLoading && hasNoPositionsOrOrders ? ( - - ) : ( - - {renderPositionsSection()} - {renderOrdersSection()} + {!isInitialLoading && hasNoPositionsOrOrders ? ( + + ) : ( + <> + + + + + {renderPositionsSection()} + {renderOrdersSection()} + - )} - - + + + )} + {isEligibilityModalVisible && ( // Android Compatibility: Wrap the in a plain component to prevent rendering issues and freezing. diff --git a/app/components/UI/Perps/components/PerpsLoadingSkeleton/PerpsLoadingSkeleton.tsx b/app/components/UI/Perps/components/PerpsLoadingSkeleton/PerpsLoadingSkeleton.tsx index ddfa58cef37e..1d3983e969d7 100644 --- a/app/components/UI/Perps/components/PerpsLoadingSkeleton/PerpsLoadingSkeleton.tsx +++ b/app/components/UI/Perps/components/PerpsLoadingSkeleton/PerpsLoadingSkeleton.tsx @@ -7,9 +7,9 @@ import { BoxAlignItems, BoxJustifyContent, TextColor, - Button, - ButtonSize, ButtonVariant, + ButtonSize, + Button, } from '@metamask/design-system-react-native'; import { useTheme } from '../../../../../util/theme'; import { strings } from '../../../../../../locales/i18n'; @@ -91,8 +91,8 @@ const PerpsLoadingSkeleton: React.FC = ({ {/* Retry Button */}