Skip to content

Commit

Permalink
fix: Provide selector that enables cross-chain polling, regardless of…
Browse files Browse the repository at this point in the history
… network filter state (#28662)

## **Description**

We cannot rely on the same selector for all cases, as not all UI is
tightly coupled to the tokenNetworkFilter, else we will not be able to
compute aggregated balances across chains, when filtered by current
network. Since polling for balances is UI based, we can use a different
selector on the network-filter, which should execute polling loops only
when the dropdown is toggled open.

With the current behavior, the aggregated balance will only display when
"All Networks" filter is selected, and when the "Current Network" is
selected, it will aggregate balances only for that chain.

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

## **Related issues**

Fixes: Current chain aggregated balance showing up in cross chain
aggregated balance when current network is filterd.

## **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] -->

## **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/develop/.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/develop/.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 Nov 26, 2024
1 parent 2a06244 commit c272b25
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import {
getCurrentChainId,
getCurrentNetwork,
getPreferences,
getChainIdsToPoll,
getShouldHideZeroBalanceTokens,
getSelectedAccount,
getAllChainsToPoll,
} from '../../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks';
import { useI18nContext } from '../../../../../hooks/useI18nContext';
Expand Down Expand Up @@ -50,7 +50,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
const shouldHideZeroBalanceTokens = useSelector(
getShouldHideZeroBalanceTokens,
);
const allChainIDs = useSelector(getChainIdsToPoll);
const allChainIDs = useSelector(getAllChainsToPoll);
const { formattedTokensWithBalancesPerChain } = useGetFormattedTokensPerChain(
selectedAccount,
shouldHideZeroBalanceTokens,
Expand Down
36 changes: 36 additions & 0 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,42 @@ export const getAllEnabledNetworks = createDeepEqualSelector(
),
);

/*
* USE THIS WITH CAUTION
*
* Only use this selector if you are absolutely sure that your UI component needs
* data from _all chains_ to compute a value. Else, use `getChainIdsToPoll`.
*
* Examples:
* - Components that should NOT use this selector:
* - Token list: This only needs to poll for chains based on the network filter
* (potentially only one chain). In this case, use `getChainIdsToPoll`.
* - Components that SHOULD use this selector:
* - Aggregated balance: This needs to display data regardless of network filter
* selection (always showing aggregated balances across all chains).
*
* Key Considerations:
* - This selector can cause expensive computations. It should only be used when
* necessary, and where possible, optimized to use `getChainIdsToPoll` instead.
* - Logic Overview:
* - If `PORTFOLIO_VIEW` is not enabled, the selector returns only the `currentChainId`.
* - Otherwise, it includes all chains from `networkConfigurations`, excluding
* `TEST_CHAINS`, while ensuring the `currentChainId` is included.
*/
export const getAllChainsToPoll = createDeepEqualSelector(
getNetworkConfigurationsByChainId,
getCurrentChainId,
(networkConfigurations, currentChainId) => {
if (!process.env.PORTFOLIO_VIEW) {
return [currentChainId];
}

return Object.keys(networkConfigurations).filter(
(chainId) => chainId === currentChainId || !TEST_CHAINS.includes(chainId),
);
},
);

export const getChainIdsToPoll = createDeepEqualSelector(
getNetworkConfigurationsByChainId,
getCurrentChainId,
Expand Down

0 comments on commit c272b25

Please sign in to comment.