Skip to content

Sort token pickers by USD value via TokenList - #2830

Merged
JakeUrban merged 2 commits into
masterfrom
plan-do-review/issue-2820
Jun 5, 2026
Merged

Sort token pickers by USD value via TokenList#2830
JakeUrban merged 2 commits into
masterfrom
plan-do-review/issue-2820

Conversation

@JakeUrban

Copy link
Copy Markdown
Contributor

Proposal: Apply value sort to token pickers via TokenList

Problem

The Account view sorts token balances by descending USD value (via sortBalancesByValue in popup/helpers/balance.ts). The Send flow's token picker (SendDestinationAsset) and the Swap flow's token picker (SwapAsset) both render tokens in sortBalances order — XLM first, LP shares last, everything else in API insertion order. This is not value-sorted and is inconsistent with the Account view.

Solution

Centralize the value-sort inside TokenList itself. TokenList is exclusively a picker component — it is only used by SendDestinationAsset (popup/components/send/SendDestinationAsset/index.tsx) and SwapAsset (popup/components/swap/SwapAsset/index.tsx), never by the Account view (which has its own rendering path via AccountAssets + useStableSortedBalances). Since TokenList already receives tokenPrices as a prop and is the single shared picker renderer, it is the natural place to own the picker ordering policy.

Why TokenList and not a shared hook? Putting the sort in a hook or helper that callers must remember to invoke is the same pattern that produced this bug — the sort existed (sortBalancesByValue), but nobody called it in the picker paths. Placing it inside TokenList makes correct ordering structural: every picker gets it automatically, and no future caller can accidentally skip it. The Account view is unaffected because it uses a completely separate rendering path (AccountAssets component with useStableSortedBalances).

Changes

File: extension/src/popup/components/InternalTransaction/TokenList/index.tsx

  1. Import sortBalancesByValue from popup/helpers/balance.
  2. At the top of the component body, sort the incoming tokens before rendering:
    const sortedTokens = sortBalancesByValue(tokens, tokenPrices);
  3. Replace tokens with sortedTokens in the render body (the .filter().filter().map() chain).

This is a single change point — both callers automatically get value-sorted rendering with zero modifications.

Testing

New tests for TokenList:

  • Tokens render in descending USD value order when tokenPrices has entries.
  • Original order is preserved when tokenPrices is empty {} (no-op behavior).
  • Hidden assets and LP shares remain excluded regardless of sort order.

Note: sortBalancesByValue already has direct unit tests in popup/helpers/__tests__/balance.test.js covering its sorting logic, edge cases, and tie-breaking. No additional helper-level tests are needed.

Edge Cases

  • Swap search filtering: In SwapAsset, filtering only applies for search terms > 2 characters (useSwapFromData.tsx:107-127). When no search is active, the full balance list reaches TokenList; when search is active, a filtered subset reaches it. In both cases, sortBalancesByValue applied inside TokenList produces the correct descending-value order for whatever subset it receives.
  • Empty prices: sortBalancesByValue is a no-op when prices is empty or null — tokens keep their existing sortBalances order (XLM first, LP last).
  • Zero price: Assets with price "0" are treated as priced with value 0 (sorting them below assets with positive value but above unpriced assets). This matches the existing sortBalancesByValue semantics.

What this does NOT change

  • XLM pinning is not applied (per updated issue description).
  • The stable-sort behavior on the Account view is unchanged (different rendering path entirely).
  • The baseline sortBalances in useGetBalances remains (provides tie-breaking insertion order for assets without price data).

Closes #2820

Apply sortBalancesByValue inside the TokenList component so that both
the Send and Swap flow token pickers display tokens in descending USD
value order, consistent with the Account view. This centralizes the
ordering policy in the shared picker renderer, eliminating the class of
bug where a new caller of TokenList forgets to sort.

Add unit tests covering value-sorted rendering, empty-price no-op
behavior, LP share exclusion, and hidden asset filtering.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 16:08
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-443c6a896799db64a563 (SDF collaborators only — install instructions in the release description)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the Send/Swap token picker ordering with the Account view by moving USD-value-based sorting into the shared TokenList picker component, ensuring consistent ordering wherever TokenList is used.

Changes:

  • Apply sortBalancesByValue(tokens, tokenPrices) inside TokenList before rendering.
  • Update TokenList rendering logic to use the sorted token list for empty-state and row rendering.
  • Add unit tests covering value-based ordering, empty-price no-op behavior, and existing filtering rules (hidden assets + LP shares).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
extension/src/popup/components/InternalTransaction/TokenList/index.tsx Sorts picker token rows by descending USD value (via sortBalancesByValue) before applying existing filters and rendering.
extension/src/popup/components/InternalTransaction/TokenList/tests/TokenList.test.tsx Adds coverage to verify value-sort ordering, no-op behavior when prices are empty, and preservation of LP/hidden-asset filtering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CassioMG

CassioMG commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

The fix work, but there is room for improvement like memoization

Memoization: sortBalancesByValue runs on every render, and the Swap picker re-renders on each (debounced) keystroke. List sizes are small so this is negligible, but a useMemo(() => sortBalancesByValue(tokens, tokenPrices), [tokens, tokenPrices]) would make intent clear and avoid resorting on unrelated re-renders.

Wrap the sortBalancesByValue call in useMemo to avoid resorting on
unrelated re-renders (e.g., debounced keystroke re-renders in Swap).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JakeUrban
JakeUrban merged commit 9b48bcf into master Jun 5, 2026
11 checks passed
@JakeUrban
JakeUrban deleted the plan-do-review/issue-2820 branch June 5, 2026 17:22
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Apply value sort to Send flow

4 participants