Skip to content

Commit

Permalink
refactor: token-cell (#30238)
Browse files Browse the repository at this point in the history
## **Description**

This PR refactors the TokenCell component to be more chain-agnostic,
introducing several improvements:

- **Separation of Concerns**: `TokenCell` and `TokenListItem` now serve
distinct purposes, making changes and testing easier. `TokenListItem` is
used in the Asset Picker, while `TokenCell` handles tokens for the main
token list.

- **Shared Business Logic**: Introduced a `useTokenDisplayInfo` hook to
centralize shared data, reducing reliance on prop overrides while
keeping markup in their respective components.

- **Style Cleanup**: Removed styles that were not applied to the main
token list.

- **Removed `NativeToken`**: This legacy component, originally needed
for PortfolioView, is no longer required. Native and non-EVM tokens will
now be integrated into the existing token list. `useNativeTokenBalance`
is still used in `TokenList` for now but may be removed soon during
multichain integration.

- **Type & Prop Consolidation**: Simplified type definitions and
streamlined prop passing from AssetList → TokenList → TokenCell.

- **Component Decomposition**: Broke `TokenCell` into smaller,
manageable components for better control and maintainability,
eliminating excessive complexity. This uncovered a few unexpected
patterns with the previous implementation that I believe were not
correct. I've added comments to point out where. Also memoized these
smaller components to help improve performance and avoid unnecessary
re-renders.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30238?quickstart=1)

## **Related issues**

Multichain effort

## **Manual testing steps**

App should function as normal without regressions. The next PR will
integrate with the new multichain controllers, and add in the multichain
feature flag / code fences to prevent unecessary polling.

## **Screenshots/Recordings**


https://github.com/user-attachments/assets/ad7bff01-4e0d-4716-9a96-23c926b90044

## **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.
  • Loading branch information
gambinish authored Feb 13, 2025
1 parent 776ef89 commit 5899e37
Show file tree
Hide file tree
Showing 31 changed files with 973 additions and 499 deletions.
19 changes: 0 additions & 19 deletions development/circular-deps.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,6 @@
"ui/components/app/alert-system/confirm-alert-modal/index.tsx",
"ui/pages/confirmations/components/confirm/footer/footer.tsx"
],
[
"ui/components/app/assets/asset-list/asset-list-control-bar/asset-list-control-bar.tsx",
"ui/components/app/assets/asset-list/asset-list-control-bar/index.ts",
"ui/components/app/assets/asset-list/asset-list.tsx",
"ui/components/app/assets/asset-list/network-filter/index.ts",
"ui/components/app/assets/asset-list/network-filter/network-filter.tsx",
"ui/hooks/useAccountTotalCrossChainFiatBalance.ts"
],
[
"ui/components/app/assets/asset-list/asset-list.tsx",
"ui/components/app/assets/asset-list/native-token/index.ts",
"ui/components/app/assets/asset-list/native-token/native-token.tsx"
],
[
"ui/components/app/assets/asset-list/asset-list.tsx",
"ui/components/app/assets/asset-list/native-token/index.ts",
"ui/components/app/assets/asset-list/native-token/native-token.tsx",
"ui/components/app/assets/asset-list/native-token/use-native-token-balance.ts"
],
[
"ui/components/app/name/name-details/name-details.tsx",
"ui/components/app/name/name.tsx"
Expand Down
36 changes: 7 additions & 29 deletions ui/components/app/assets/asset-list/asset-list.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useContext, useMemo } from 'react';
import React, { useCallback, useContext } from 'react';
import { useSelector } from 'react-redux';
import TokenList from '../token-list';
import { getMultichainIsEvm } from '../../../../selectors/multichain';
Expand All @@ -8,23 +8,14 @@ import {
MetaMetricsEventName,
} from '../../../../../shared/constants/metametrics';
import DetectedToken from '../../detected-token/detected-token';
import useAssetListTokenDetection from '../hooks/useAssetListTokenDetection';
import usePrimaryCurrencyProperties from '../hooks/usePrimaryCurrencyProperties';
import {
useAssetListTokenDetection,
usePrimaryCurrencyProperties,
} from '../hooks';
import AssetListControlBar from './asset-list-control-bar';
import NativeToken from './native-token';
import AssetListFundingModals from './asset-list-funding-modals';

export type TokenWithBalance = {
address: string;
symbol: string;
string?: string;
image: string;
secondary?: string;
tokenFiatAmount?: string;
isNative?: boolean;
};

export type AssetListProps = {
type AssetListProps = {
onClickAsset: (chainId: string, address: string) => void;
showTokensLinks?: boolean;
};
Expand All @@ -33,7 +24,6 @@ const TokenListContainer = React.memo(
({ onClickAsset }: Pick<AssetListProps, 'onClickAsset'>) => {
const trackEvent = useContext(MetaMetricsContext);
const { primaryCurrencyProperties } = usePrimaryCurrencyProperties();
const isEvm = useSelector(getMultichainIsEvm);

const onTokenClick = useCallback(
(chainId: string, tokenAddress: string) => {
Expand All @@ -50,19 +40,7 @@ const TokenListContainer = React.memo(
[],
);

const nativeToken = useMemo(
() => !isEvm && <NativeToken onClickAsset={onClickAsset} />,
[isEvm, onClickAsset],
);

return (
<TokenList
// nativeToken is still needed to avoid breaking flask build's support for bitcoin
// TODO: refactor this to no longer be needed for non-evm chains
nativeToken={nativeToken}
onTokenClick={onTokenClick}
/>
);
return <TokenList onTokenClick={onTokenClick} />;
},
);

Expand Down
1 change: 0 additions & 1 deletion ui/components/app/assets/asset-list/native-token/index.ts

This file was deleted.

50 changes: 0 additions & 50 deletions ui/components/app/assets/asset-list/native-token/native-token.tsx

This file was deleted.

5 changes: 5 additions & 0 deletions ui/components/app/assets/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export { default as useAssetListTokenDetection } from './useAssetListTokenDetection';
export { default as useNativeTokenBalance } from './useNativeTokenBalance';
export { default as useNetworkFilter } from './useNetworkFilter';
export { default as usePrimaryCurrencyProperties } from './usePrimaryCurrencyProperties';
export { default as useTokenDisplayInfo } from './useTokenDisplayInfo';
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import currencyFormatter from 'currency-formatter';
import { useSelector } from 'react-redux';

import { Hex } from '@metamask/utils';
import {
getMultichainCurrencyImage,
getMultichainCurrentNetwork,
getMultichainSelectedAccountCachedBalance,
getMultichainShouldShowFiat,
} from '../../../../../selectors/multichain';
} from '../../../../selectors/multichain';
import {
getPreferences,
getSelectedInternalAccount,
} from '../../../../../selectors';
import { getCurrentCurrency } from '../../../../../ducks/metamask/metamask';
import { useIsOriginalNativeTokenSymbol } from '../../../../../hooks/useIsOriginalNativeTokenSymbol';
import { PRIMARY, SECONDARY } from '../../../../../helpers/constants/common';
import { useUserPreferencedCurrency } from '../../../../../hooks/useUserPreferencedCurrency';
import { useCurrencyDisplay } from '../../../../../hooks/useCurrencyDisplay';
import { TokenWithBalance } from '../asset-list';
} from '../../../../selectors';
import { getCurrentCurrency } from '../../../../ducks/metamask/metamask';
import { useIsOriginalNativeTokenSymbol } from '../../../../hooks/useIsOriginalNativeTokenSymbol';
import { PRIMARY, SECONDARY } from '../../../../helpers/constants/common';
import { useUserPreferencedCurrency } from '../../../../hooks/useUserPreferencedCurrency';
import { useCurrencyDisplay } from '../../../../hooks/useCurrencyDisplay';
import { TokenWithFiatAmount } from '../types';

export const useNativeTokenBalance = () => {
const useNativeTokenBalance = () => {
const showFiat = useSelector(getMultichainShouldShowFiat);
const account = useSelector(getSelectedInternalAccount);
const primaryTokenImage = useSelector(getMultichainCurrencyImage);
Expand Down Expand Up @@ -81,21 +81,26 @@ export const useNativeTokenBalance = () => {
// useCurrencyDisplay passes along the symbol and formatting into the value here
// for sorting we need the raw value, without the currency and it should be decimal
// this is the easiest way to do this without extensive refactoring of useCurrencyDisplay
const tokenFiatAmount = currencyFormatter
.unformat(unformattedTokenFiatAmount, {
const tokenFiatAmount = currencyFormatter.unformat(
unformattedTokenFiatAmount,
{
code: currentCurrency.toUpperCase(),
})
.toString();
},
);

const nativeTokenWithBalance: TokenWithBalance = {
address: '',
const nativeTokenWithBalance: TokenWithFiatAmount = {
chainId: chainId as Hex,
address: '' as Hex,
symbol: tokenSymbol ?? '',
string: primaryBalance,
image: primaryTokenImage,
secondary: secondaryBalance,
tokenFiatAmount,
isNative: true,
decimals: 18,
};

return nativeTokenWithBalance;
};

export default useNativeTokenBalance;
24 changes: 0 additions & 24 deletions ui/components/app/assets/hooks/useShouldShowFiat.tsx

This file was deleted.

70 changes: 0 additions & 70 deletions ui/components/app/assets/hooks/useSortedFilteredTokens.tsx

This file was deleted.

Loading

0 comments on commit 5899e37

Please sign in to comment.