refactor asset-list-item to use list-item component #8725
Conversation
Refactors the asset-list-item and token-cell to rely on the list-item component for UI. Little changes were needed to the list-item code to make this work! The result should be lots of eliminated code
Builds ready [0365fae]
Page Load Metrics (586 ± 58 ms)
|
| numberOfDecimals: 2, | ||
| conversionRate: currentTokenToFiatRate, | ||
| }) | ||
| formattedFiat = `${formatCurrency(currentTokenInFiat, currentCurrency)} ${currentCurrency.toUpperCase()}` |
There was a problem hiding this comment.
Previously this would omit fiat altogether if the value was zero, e.g. $0.
I think that's still something we want to do? 🤔
There was a problem hiding this comment.
Speaking only from my personal preference: I like having it say $0.00 when token quantity is 0. It creates symmetry with other non zero balance tokens.
There was a problem hiding this comment.
Even when the conversion rate is missing? A side-effect of the old logic was that the fiat was hidden in the case where the information required to convert it was missing.
Showing $0.00 for a token worth a non-zero amount does seem misleading. Perhaps you could update this to hide the fiat balance if the conversion rate is missing, instead of when it's zero.
There was a problem hiding this comment.
if the conversion rate is missing, currentTokenInFiat never gets set. My assumption was showFiat would be false when it tries to evaluate Boolean(currentTokenInFiat) in this case. Is that not correct?
There was a problem hiding this comment.
oh, are you saying conversionRate is missing not contractExchangeRates[address]?
There was a problem hiding this comment.
Yeah, currentTokenToFiatRate in particular. I thought that the conversion rate defaulted to zero if it wasn't given? 🤔 Maybe that's not the case.
There was a problem hiding this comment.
Ah, apparently it's always fetched but it starts out as zero: https://github.com/MetaMask/controllers/blob/develop/src/assets/CurrencyRateController.ts#L79
There was a problem hiding this comment.
Ah, got it. What if i changed:
if (contractExchangeRates[address]) {
to
if (conversionRate > 0 && contractExchangeRates[address]) {
Co-authored-by: Mark Stacey <[email protected]>
Builds ready [17f645b]
Page Load Metrics (842 ± 88 ms)
|
Builds ready [1a79873]
Page Load Metrics (626 ± 43 ms)
|
Refactors the asset-list-item and token-cell to rely on the list-item
component for UI. Little changes were needed to the list-item code
to make this work!