Conversation
…eria, defaulted with 100 results.
…ntend-monorepo into feat/joinable-pools
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| isTokenInWallet={ | ||
| joinablePools | ||
| ? tokenAddress => hasWalletTokenBalance(pool.chain, tokenAddress) | ||
| : undefined |
There was a problem hiding this comment.
shouldn't this be either true or false? what does undefined mean here?
There was a problem hiding this comment.
having a balance for a token in the wallet defines the highlight styling in the pills
so a function is passed for that purpose
we can't check this in PoolTokenPills itself because it's also used outside of the PoolList context
| }), | ||
| }) | ||
|
|
||
| const walletTokenAddressesByChain = useMemo(() => { |
There was a problem hiding this comment.
Not sure why do we need the useMemo here. Normally it is used when calculations are expensive (e.g. network calls), but doesn't seem to be the case here.
There was a problem hiding this comment.
there is some looping and formatting done so i think it's ok to keep it
| }, [joinablePools, joinableChains, walletBalanceQueries, priceFor]) | ||
|
|
||
| const joinablePoolsQuery = useReactQuery({ | ||
| queryKey: [ |
There was a problem hiding this comment.
do we really want to cache each of these queries?
There was a problem hiding this comment.
it's caching several apollo queries so why don't do it?
| staleTime: 30_000, | ||
| }) | ||
|
|
||
| const joinablePoolsData = useMemo(() => { |
There was a problem hiding this comment.
not sure we need the useMemo here.
There was a problem hiding this comment.
there is some deduping and sorting done so i think it's ok to keep it
| count: joinablePools ? filteredPools.length : data?.count || previousData?.count, | ||
| queryState, | ||
| loading, | ||
| loading: loading || (joinablePools && isJoinableBalanceLoading), |
There was a problem hiding this comment.
isJoinableBalanceLoading already has joinablePools as conditions, this seems like a duplicate.
| tokens: (PoolToken | ApiToken)[] | ||
| chain: GqlChain | ||
| iconSize?: number | ||
| isTokenInWallet?: IsTokenInWallet |
There was a problem hiding this comment.
what is the idea of passing this function as a parameter here? (having this IsTokenInWallet type looks really strange)
| <Badge | ||
| key={[token.address, token.chain, i].join('-')} | ||
| {...badgeProps} | ||
| _after={ |
There was a problem hiding this comment.
All this block seems repeated before, can we abstract this somehow?
| const normalizedJoinablePoolsValue = joinablePoolsValue?.toLowerCase() | ||
| const joinablePools = | ||
| normalizedJoinablePoolsValue !== undefined && | ||
| ['true', '1', ''].includes(normalizedJoinablePoolsValue) |
There was a problem hiding this comment.
don't understand this code. What does normalizedJoinablePoolsValue stand for? why can it be so many different kind of values?
There was a problem hiding this comment.
updated to only use joinablePools=true in the url when the filter is active
| getRowId, | ||
| loadingLength = 20, | ||
| paginationStyles, | ||
| loadingSpinnerPosition = 'center', |
There was a problem hiding this comment.
why do we want the spinner position to be different on PaginatedTable's? shouldn't all of them work in the same way?
3 participants
No description provided.