-
Notifications
You must be signed in to change notification settings - Fork 4
feat(token-select): sort by positive balance by default when wallet connected #475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
165c167
ebb18fb
f1a8adc
9f5b773
d8c5ae2
0897b15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { render, screen } from '@testing-library/react' | |
| import type { ReactNode } from 'react' | ||
| import { createElement } from 'react' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import type { Web3Status } from '@/src/hooks/useWeb3Status' | ||
| import SignButton from './SignButton' | ||
|
|
||
| const mockSwitchChain = vi.fn() | ||
|
|
@@ -45,6 +46,21 @@ vi.mock('wagmi', () => ({ | |
| const { useWalletStatus } = await import('@/src/hooks/useWalletStatus') | ||
| const mockedUseWalletStatus = vi.mocked(useWalletStatus) | ||
|
|
||
| const mockWeb3Status = { | ||
| readOnlyClient: undefined, | ||
| appChainId: 1, | ||
| address: '0xdeadbeef', | ||
| balance: undefined, | ||
| connectingWallet: false, | ||
| switchingChain: false, | ||
| isWalletConnected: true, | ||
| walletClient: undefined, | ||
| isWalletSynced: true, | ||
| walletChainId: 1, | ||
| switchChain: vi.fn(), | ||
| disconnect: vi.fn(), | ||
| } as unknown as Web3Status | ||
|
|
||
| const system = createSystem(defaultConfig) | ||
|
|
||
| const renderWithChakra = (ui: ReactNode) => | ||
|
|
@@ -63,6 +79,7 @@ describe('SignButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
|
||
| renderWithChakra(<SignButton message="Hello" />) | ||
|
|
@@ -79,6 +96,7 @@ describe('SignButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
96
to
100
|
||
|
|
||
| renderWithChakra( | ||
|
|
@@ -102,6 +120,7 @@ describe('SignButton', () => { | |
| >['targetChain'], | ||
| targetChainId: 10, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
120
to
124
|
||
|
|
||
| renderWithChakra(<SignButton message="Hello" />) | ||
|
|
@@ -119,6 +138,7 @@ describe('SignButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
138
to
142
|
||
|
|
||
| renderWithChakra(<SignButton message="Hello" />) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,7 @@ export interface TokenSelectProps { | |||||||
| showAddTokenButton?: boolean | ||||||||
| showTopTokens?: boolean | ||||||||
| showBalance?: boolean | ||||||||
| sortByBalance?: boolean | ||||||||
| } | ||||||||
|
|
||||||||
| /** @ignore */ | ||||||||
|
|
@@ -42,8 +43,9 @@ type Props = FlexProps & TokenSelectProps | |||||||
| * @param {number} [props.iconSize=32] - Optional size of the token icon in the list. Default is 32. | ||||||||
| * @param {number} [props.itemHeight=64] - Optional height of each item in the list. Default is 64. | ||||||||
| * @param {boolean} [props.showAddTokenButton=false] - Optional flag to allow adding a token. Default is false. | ||||||||
| * @param {boolean} [props.showBalance=false] - Optional flag to show the token balance in the list. Default is false. | ||||||||
| * @param {boolean} [props.showBalance=false] - Optional flag to show the token balance column in each row. Default is false. | ||||||||
| * @param {boolean} [props.showTopTokens=false] - Optional flag to show the top tokens in the list. Default is false. | ||||||||
| * @param {boolean} [props.sortByBalance] - Sort tokens with a positive balance to the top, ordered by USD value descending. Defaults to true when a wallet is connected. | ||||||||
| */ | ||||||||
| const TokenSelect = withSuspenseAndRetry<Props>( | ||||||||
| ({ | ||||||||
|
|
@@ -59,9 +61,10 @@ const TokenSelect = withSuspenseAndRetry<Props>( | |||||||
| showAddTokenButton = false, | ||||||||
| showBalance = false, | ||||||||
| showTopTokens = false, | ||||||||
| sortByBalance, | ||||||||
| ...restProps | ||||||||
| }) => { | ||||||||
| const { appChainId, walletChainId } = useWeb3Status() | ||||||||
| const { appChainId, isWalletConnected, walletChainId } = useWeb3Status() | ||||||||
|
|
||||||||
| const [chainId, setChainId] = useState<Chain['id']>(() => | ||||||||
| getValidChainId({ | ||||||||
|
|
@@ -122,9 +125,12 @@ const TokenSelect = withSuspenseAndRetry<Props>( | |||||||
| previousDepsRef.current = [appChainId, currentNetworkId, walletChainId] | ||||||||
| }, [appChainId, currentNetworkId, networks, walletChainId]) | ||||||||
|
|
||||||||
| const resolvedSortByBalance = sortByBalance ?? isWalletConnected | ||||||||
|
|
||||||||
| const { isLoadingBalances, tokensByChainId } = useTokens({ | ||||||||
| chainId, | ||||||||
| withBalance: showBalance, | ||||||||
| withBalance: showBalance || resolvedSortByBalance, | ||||||||
|
||||||||
| withBalance: showBalance || resolvedSortByBalance, | |
| withBalance: showBalance || resolvedSortByBalance, | |
| sortByBalance: resolvedSortByBalance, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0897b15. Added a sortByBalance parameter (default true) to useTokens, updateTokensBalances, and updateTokensWithRawBalances; the sort step is now skipped when false. TokenSelect passes sortByBalance: resolvedSortByBalance, so sortByBalance={false} with showBalance={true} now shows balances without reordering.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { ChakraProvider, createSystem, defaultConfig } from '@chakra-ui/react' | |
| import { render, screen } from '@testing-library/react' | ||
| import { createElement, type ReactNode } from 'react' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import type { Web3Status } from '@/src/hooks/useWeb3Status' | ||
| import TransactionButton from './TransactionButton' | ||
|
|
||
| const mockSwitchChain = vi.fn() | ||
|
|
@@ -43,6 +44,21 @@ vi.mock('wagmi', () => ({ | |
| const { useWalletStatus } = await import('@/src/hooks/useWalletStatus') | ||
| const mockedUseWalletStatus = vi.mocked(useWalletStatus) | ||
|
|
||
| const mockWeb3Status = { | ||
| readOnlyClient: undefined, | ||
| appChainId: 1, | ||
| address: '0xdeadbeef', | ||
| balance: undefined, | ||
| connectingWallet: false, | ||
| switchingChain: false, | ||
| isWalletConnected: true, | ||
| walletClient: undefined, | ||
| isWalletSynced: true, | ||
| walletChainId: 1, | ||
| switchChain: vi.fn(), | ||
| disconnect: vi.fn(), | ||
| } as unknown as Web3Status | ||
|
|
||
| const system = createSystem(defaultConfig) | ||
|
|
||
| const renderWithChakra = (ui: ReactNode) => | ||
|
|
@@ -61,6 +77,7 @@ describe('TransactionButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
77
to
81
|
||
|
|
||
| renderWithChakra(<TransactionButton transaction={mockTransaction}>Send</TransactionButton>) | ||
|
|
@@ -77,6 +94,7 @@ describe('TransactionButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
94
to
98
|
||
|
|
||
| renderWithChakra( | ||
|
|
@@ -102,6 +120,7 @@ describe('TransactionButton', () => { | |
| >['targetChain'], | ||
| targetChainId: 10, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
120
to
124
|
||
|
|
||
| renderWithChakra(<TransactionButton transaction={mockTransaction}>Send</TransactionButton>) | ||
|
|
@@ -121,6 +140,7 @@ describe('TransactionButton', () => { | |
| >['targetChain'], | ||
| targetChainId: 10, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
140
to
144
|
||
|
|
||
| renderWithChakra( | ||
|
|
@@ -144,6 +164,7 @@ describe('TransactionButton', () => { | |
| targetChain: { id: 1, name: 'Ethereum' } as ReturnType<typeof useWalletStatus>['targetChain'], | ||
| targetChainId: 1, | ||
| switchChain: mockSwitchChain, | ||
| web3Status: mockWeb3Status, | ||
| }) | ||
|
Comment on lines
164
to
168
|
||
|
|
||
| renderWithChakra(<TransactionButton transaction={mockTransaction}>Send ETH</TransactionButton>) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
useWalletStatusmock returnsweb3Statusasundefinedvia a cast. SinceuseWalletStatusnow guarantees a realWeb3Statusobject, this mock can hide runtime issues if the component starts usingweb3Status. Prefer returning a minimal but validWeb3Statusstub instead of castingundefined.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d8c5ae2. Replaced the
undefined as unknowncast with a sharedmockWeb3Statusstub mirroringWalletStatusVerifier.test.tsx.