Uh oh!
There was an error while loading. Please reload this page.
Update swap flows for artist coins - #12643
Conversation
|
🚨 Bugbot Trial ExpiredYour Bugbot trial has expired. Please purchase a license in the Cursor dashboard to continue using Bugbot. |
Uh oh!
There was an error while loading. Please reload this page.
| <Text variant='body' size='s' color='subdued'> | ||
| {messages.exchangeRateLabel} | ||
| </Text> | ||
| <Text variant='body' size='s' color='default'> |
There was a problem hiding this comment.
nit: think color='default' not needed
| ) : null} | ||
| {hasReceiveAmount && | ||
| formattedReceiveAmount !== DEFAULT_TOKEN_AMOUNT_PLACEHOLDER ? ( | ||
| <Flex direction='column'> |
| > | ||
| <Flex flex={1} alignItems='center' justifyContent='space-between'> | ||
| <Flex column gap='xs'> | ||
| <Text variant='heading' size='m' color='default'> |
| <Flex column> | ||
| {coinPairs.map((pair, rowIndex) => ( | ||
| <> | ||
| <Flex key={`row-${rowIndex}`} alignItems='stretch'> |
There was a problem hiding this comment.
isn't there something about not using the index in keys? can we use the token name or something instead
There was a problem hiding this comment.
since a pair can either be a coin or the find-more row, this is the most simple specific thing we can go with for now
There was a problem hiding this comment.
https://robinpokorny.com/blog/index-as-a-key-is-an-anti-pattern/
Update: Exception from the rule
Many people asked if they always, always have to generate ids. Others have suggested use cases when using the index as a key seems justifiable.
It is true that sometimes generating new ids is redundant and may be avoided. For example translation of license terms or list of contributors.
To help you decide, I put together three conditions which these examples have in common:
the list and items are static–they are not computed and do not change;
the items in the list have no ids;
the list is never reordered or filtered.
When all of them are met, you may safely use the index as a key.
There was a problem hiding this comment.
i guess in this case probably ok, cc @dylanjeffers
There was a problem hiding this comment.
fine, but prob chill to just have the key={coin} ?
There was a problem hiding this comment.
added a better key for the row items, but we can't really get a clearer key for the rows themselves without doing something overly complicated
Uh oh!
There was an error while loading. Please reload this page.
| ) | ||
| })} | ||
| <Flex column> | ||
| {coinPairs.map((pair, rowIndex) => ( |
There was a problem hiding this comment.
this whole coinPairs thing seems kinda confusing/overkill.. i don't have a better solution off the top of my head but it seems to me like there should be a way to just do a list map over the coins and spread them into a grid, maybe css grid could be helpful here?
no need to change now but let's keep thinking
There was a problem hiding this comment.
agree, was having some problems with grid so chose this route for now
🚨 Bugbot Trial ExpiredYour Bugbot trial has expired. Please purchase a license in the Cursor dashboard to continue using Bugbot. |
| coinPairs.push(['find-more']) | ||
| } | ||
| return coinPairs |
| // Group coins into pairs for row rendering | ||
| const coinPairs: CoinPairItem[][] = [] | ||
| for (let i = 0; i < filteredCoins.length; i += 2) { |
There was a problem hiding this comment.
could this be used in useGroupCoinPairs?
Uh oh!
There was an error while loading. Please reload this page.
Bugbot found 3 bugsTo see them, activate your membership in the Cursor dashboard. |
Description
This PR updates the swap flows for artist coins introducing things such as the rate on each tab. It also correctly displays the owned assets in the "Your Coins" section
How Has This Been Tested?
npm run web:prod