fix missing conversion rates in swaps token drop down #12420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
adonesky1
force-pushed
the
fix/missing-conversion-rates-swaps-dropdown
branch
from
068ee11
to
6b22e5a
Compare
Builds ready [c674179]
Page Load Metrics (421 ± 55 ms)
|
darkwing
reviewed
Oct 21, 2021
ui/hooks/useTokensToSearch.js
Outdated
| @@ -172,6 +172,10 @@ export function useTokensToSearch({ | |||
| return new BigNumber(rawFiat || 0).gt(secondRawFiat || 0) ? -1 : 1; | |||
| }, | |||
| ); | |||
|
|
|||
| // the defaultToken/asset should always be at the top of the owned section | |||
| tokensToSearchBuckets.owned.unshift(memoizedDefaultToken); | |||
There was a problem hiding this comment.
Nice! My only concern was if owned was ever not an array, and ui/hooks/useTokensToSearch.js shows it initialized as empty, so we're good!
darkwing
previously approved these changes
Oct 21, 2021
adonesky1
force-pushed
the
fix/missing-conversion-rates-swaps-dropdown
branch
from
c674179
to
6d1876b
Compare
…own with full renderable data regardless of whether swaps tokens are available
adonesky1
force-pushed
the
fix/missing-conversion-rates-swaps-dropdown
branch
from
6d1876b
to
a5171c4
Compare
adonesky1
commented
Oct 21, 2021
| @@ -139,7 +142,7 @@ export function useTokensToSearch({ | |||
| }; | |||
|
|
|||
| const memoizedSwapsAndUserTokensWithoutDuplicities = uniqBy( | |||
| [...memoizedTokensToSearch, ...memoizedUsersToken], | |||
| [memoizedDefaultToken, ...memoizedTokensToSearch, ...memoizedUsersToken], | |||
There was a problem hiding this comment.
I found that this change was required to get the native asset to consistently show conversionRate as well. Otherwise we would use the swapsTokens instance of ETH (when swapsTokens are available, which appears to be intermittent?) which has no balance information resulting in not showing conversions. cc @dan437 @danjm
Builds ready [a5171c4]
Page Load Metrics (497 ± 59 ms)
|
adonesky1
commented
Oct 21, 2021
dan437
approved these changes
Oct 22, 2021
Builds ready [8272efd]
Page Load Metrics (453 ± 65 ms)
|
darkwing
approved these changes
Oct 22, 2021
|
@danjm this okay to merge and pull into 10.3.0? Does it resolve all concerns? |
jpuri
approved these changes
Oct 22, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation: Conversion rates were missing in the swaps "from token" searchable dropdown because contractExchangeRates are keyed with addresses in checksum format. This is yet another case where address format normalization needs to take place globally
additionally the defaultToken (ETH) appeared to only render conversionRates when
swapsTokenswere unavailable so this PR alters logic for deriving the tokensToSearchBuckets such that we guarantee the defaultToken/native asset has data required to show fiat conversion available.