Skip to content

Commit

Permalink
fix: fix sort feature (#13277)
Browse files Browse the repository at this point in the history 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
Fix a regression on sort token feature
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

## **Related issues**

Fixes: #13260 

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->



https://github.com/user-attachments/assets/b9ccdfd4-6a9f-47b9-93f6-ec2687487158



## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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
@salimtb
salimtb authored Jan 30, 2025
1 parent b5992b0 commit c28efe4
Show file tree
Hide file tree
Showing 6 changed files with 235 additions and 124 deletions.
164 changes: 82 additions & 82 deletions app/components/UI/Tokens/__snapshots__/index.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,32 +476,32 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
tokens={
[
{
"address": "0x0",
"address": "0x01",
"balanceFiat": "",
"chainId": "0x1",
"decimals": 18,
"iconUrl": "",
"isETH": false,
"isNative": false,
"isStaked": false,
"name": "Ethereum",
"symbol": "ETH",
"token": "Ethereum",
"name": "Bat",
"symbol": "BAT",
"token": "Bat",
"tokenFiatAmount": 0,
},
{
"address": "0x01",
"address": "0x0",
"balanceFiat": "",
"chainId": "0x1",
"decimals": 18,
"iconUrl": "",
"isETH": false,
"isNative": false,
"isStaked": false,
"name": "Bat",
"symbol": "BAT",
"token": "Bat",
"tokenFiatAmount": 0,
"name": "Ethereum",
"symbol": "ETH",
"token": "Ethereum",
"tokenFiatAmount": undefined,
},
]
}
Expand All @@ -510,32 +510,32 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
data={
[
{
"address": "0x0",
"address": "0x01",
"balanceFiat": "",
"chainId": "0x1",
"decimals": 18,
"iconUrl": "",
"isETH": false,
"isNative": false,
"isStaked": false,
"name": "Ethereum",
"symbol": "ETH",
"token": "Ethereum",
"name": "Bat",
"symbol": "BAT",
"token": "Bat",
"tokenFiatAmount": 0,
},
{
"address": "0x01",
"address": "0x0",
"balanceFiat": "",
"chainId": "0x1",
"decimals": 18,
"iconUrl": "",
"isETH": false,
"isNative": false,
"isStaked": false,
"name": "Bat",
"symbol": "BAT",
"token": "Bat",
"tokenFiatAmount": 0,
"name": "Ethereum",
"symbol": "ETH",
"token": "Ethereum",
"tokenFiatAmount": undefined,
},
]
}
Expand Down Expand Up @@ -587,7 +587,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
"paddingVertical": 10,
}
}
testID="asset-ETH"
testID="asset-BAT"
>
<View
onLayout={[Function]}
Expand Down Expand Up @@ -738,7 +738,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
}
>
Ethereum
Bat
</Text>
</View>
<View>
Expand Down Expand Up @@ -809,35 +809,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
testID="fiat-balance-test-id"
>
<View
style={
[
{
"backgroundColor": "#f2f4f6",
"borderRadius": 30,
"padding": 14,
},
{
"padding": 8,
},
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
{
"width": 50,
},
]
}
/>
&lt; 0.00001 BAT
</Text>
<Text
accessibilityRole="text"
Expand All @@ -855,35 +827,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
testID="main-balance-test-id"
>
<View
style={
[
{
"backgroundColor": "#f2f4f6",
"borderRadius": 30,
"padding": 14,
},
{
"padding": 8,
},
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
{
"width": 50,
},
]
}
/>
$0
</Text>
</View>
</TouchableOpacity>
Expand All @@ -905,7 +849,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
"paddingVertical": 10,
}
}
testID="asset-BAT"
testID="asset-ETH"
>
<View
onLayout={[Function]}
Expand Down Expand Up @@ -1056,7 +1000,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
}
>
Bat
Ethereum
</Text>
</View>
<View>
Expand Down Expand Up @@ -1127,7 +1071,35 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
testID="fiat-balance-test-id"
>
&lt; 0.00001 BAT
<View
style={
[
{
"backgroundColor": "#f2f4f6",
"borderRadius": 30,
"padding": 14,
},
{
"padding": 8,
},
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
{
"width": 50,
},
]
}
/>
</Text>
<Text
accessibilityRole="text"
Expand All @@ -1145,7 +1117,35 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is
}
testID="main-balance-test-id"
>
$0
<View
style={
[
{
"backgroundColor": "#f2f4f6",
"borderRadius": 30,
"padding": 14,
},
{
"padding": 8,
},
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
{
"width": 50,
},
]
}
/>
</Text>
</View>
</TouchableOpacity>
Expand Down
66 changes: 25 additions & 41 deletions app/components/UI/Tokens/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,51 +207,29 @@ const Tokens: React.FC<TokensI> = memo(({ tokens }) => {
return [...nativeTokens, ...nonNativeTokens];
};

const calculateTokenFiatBalances = (assets: TokenI[]) => {
const tokenFiatBalances: number[] = [];

for (const token of assets) {
const calculateFiatBalances = (assets: TokenI[]) =>
assets.map((token) => {
const chainId = token.chainId as Hex;
const multiChainExchangeRates = debouncedMultiChainMarketData?.[chainId];
const multiChainExchangeRates = multiChainMarketData?.[chainId];
const multiChainTokenBalances =
debouncedMultiChainTokenBalance?.[
selectedInternalAccountAddress as Hex
]?.[chainId];
multiChainTokenBalance?.[selectedInternalAccountAddress as Hex]?.[
chainId
];
const nativeCurrency =
networkConfigurationsByChainId[chainId].nativeCurrency;
const multiChainConversionRate =
debouncedMultiChainCurrencyRates?.[nativeCurrency]?.conversionRate || 0;

// Calculate fiat balance for the token
const fiatBalance =
token.isETH || token.isNative
? parseFloat(token.balance) * multiChainConversionRate
: deriveBalanceFromAssetMarketDetails(
token,
multiChainTokenBalances || {},
multiChainExchangeRates || {},
multiChainConversionRate || 0,
currentCurrency || '',
).balanceFiatCalculation;

// Add the calculated balance to the array
tokenFiatBalances.push(fiatBalance || 0);
}

const tokensWithBalances: typeof assets = [];

for (let i = 0; i < assets.length; i++) {
const token = assets[i];
const tokenWithBalance = {
...token,
tokenFiatAmount: tokenFiatBalances[i],
};

tokensWithBalances.push(tokenWithBalance);
}

return tokensWithBalances;
};
multiChainCurrencyRates?.[nativeCurrency]?.conversionRate || 0;

return token.isETH || token.isNative
? parseFloat(token.balance) * multiChainConversionRate
: deriveBalanceFromAssetMarketDetails(
token,
multiChainExchangeRates || {},
multiChainTokenBalances || {},
multiChainConversionRate || 0,
currentCurrency || '',
).balanceFiatCalculation;
});

const filterTokensByNetwork = (tokensToDisplay: TokenI[]): TokenI[] => {
if (isAllNetworks && isPopularNetwork) {
Expand Down Expand Up @@ -286,7 +264,13 @@ const Tokens: React.FC<TokensI> = memo(({ tokens }) => {

const assets = categorizeTokens(filteredTokens);

const tokensWithBalances = calculateTokenFiatBalances(assets);
// Calculate fiat balances for tokens
const tokenFiatBalances = calculateFiatBalances(assets);

const tokensWithBalances = assets.map((token, i) => ({
...token,
tokenFiatAmount: tokenFiatBalances[i],
}));

const tokensSorted = sortAssets(tokensWithBalances, tokenSortConfig);
endTrace({
Expand Down
25 changes: 25 additions & 0 deletions e2e/pages/wallet/TokenSortBottomSheet.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { WalletViewSelectorsIDs } from '../../selectors/wallet/WalletView.selectors';
import Gestures from '../../utils/Gestures';
import Matchers from '../../utils/Matchers';

class SortModal {
get sortAlphabetically() {
return Matchers.getElementByID(WalletViewSelectorsIDs.SORT_ALPHABETICAL);
}

get sortFiatAmount() {
return Matchers.getElementByID(
WalletViewSelectorsIDs.SORT_DECLINING_BALANCE,
);
}

async tapSortAlphabetically() {
await Gestures.waitAndTap(this.sortAlphabetically);
}

async tapSortFiatAmount() {
await Gestures.waitAndTap(this.sortFiatAmount);
}
}

export default new SortModal();
Loading

0 comments on commit c28efe4

Please sign in to comment.