refactor asset-list-item to use list-item component (#8725)

* refactor asset items to use list-item

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

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

test/e2e/address-book.spec.js

@@ -151,7 +151,7 @@ describe('MetaMask', function () {
     })
 
     it('balance renders', async function () {
-      const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .asset-list__primary-amount'))
+      const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .list-item__heading'))
       await driver.wait(until.elementTextMatches(balance, /25\s*ETH/))
       await driver.delay(regularDelayMs)
     })

test/e2e/metamask-ui.spec.js

@@ -207,7 +207,7 @@ describe('MetaMask', function () {
     })
 
     it('balance renders', async function () {
-      const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .asset-list__primary-amount'))
+      const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .list-item__heading'))
       await driver.wait(until.elementTextMatches(balance, /100\s*ETH/))
       await driver.delay(regularDelayMs)
     })

test/e2e/threebox.spec.js

@@ -96,7 +96,7 @@ describe('MetaMask', function () {
       })
 
       it('balance renders', async function () {
-        const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .asset-list__primary-amount'))
+        const balance = await driver.findElement(By.css('[data-testid="wallet-balance"] .list-item__heading'))
         await driver.wait(until.elementTextMatches(balance, /25\s*ETH/))
         await driver.delay(regularDelayMs)
       })
@@ -202,7 +202,7 @@ describe('MetaMask', function () {
       })
 
       it('balance renders', async function () {
-        const balance = await driver2.findElement(By.css('[data-testid="wallet-balance"] .asset-list__primary-amount'))
+        const balance = await driver2.findElement(By.css('[data-testid="wallet-balance"] .list-item__heading'))
         await driver2.wait(until.elementTextMatches(balance, /25\s*ETH/))
         await driver2.delay(regularDelayMs)
       })

ui/app/components/app/asset-list-item/asset-list-item.js

@@ -2,9 +2,9 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import classnames from 'classnames'
 import Identicon from '../../ui/identicon'
+import ListItem from '../../ui/list-item'
 
 const AssetListItem = ({
-  children,
   className,
   'data-testid': dataTestId,
   iconClassName,
@@ -12,32 +12,31 @@ const AssetListItem = ({
   tokenAddress,
   tokenImage,
   warning,
+  primary,
+  secondary,
 }) => {
   return (
-    <div
+    <ListItem
-      className={classnames('asset-list-item__container', className)}
+      className={classnames('asset-list-item', className)}
       data-testid={dataTestId}
+      title={primary}
+      titleIcon={warning}
+      subtitle={secondary}
       onClick={onClick}
-    >
+      icon={(
         <Identicon
           className={iconClassName}
           diameter={32}
           address={tokenAddress}
           image={tokenImage}
         />
-      <div
+      )}
-        className="asset-list-item__balance"
+      rightContent={<i className="fas fa-chevron-right asset-list-item__chevron-right" />}
-      >
+    />
-        { children }
-      </div>
-      { warning }
-      <i className="fas fa-chevron-right asset-list-item__chevron-right" />
-    </div>
   )
 }
 
 AssetListItem.propTypes = {
-  children: PropTypes.node.isRequired,
   className: PropTypes.string,
   'data-testid': PropTypes.string,
   iconClassName: PropTypes.string,
@@ -45,6 +44,8 @@ AssetListItem.propTypes = {
   tokenAddress: PropTypes.string,
   tokenImage: PropTypes.string,
   warning: PropTypes.node,
+  primary: PropTypes.string,
+  secondary: PropTypes.string,
 }
 
 AssetListItem.defaultProps = {

ui/app/components/app/asset-list-item/asset-list-item.scss

@@ -1,26 +1,10 @@
 .asset-list-item {
-  &__container {
-    display: flex;
-    padding: 24px 16px;
-    align-items: center;
-    border-top: 1px solid $mercury;
-    border-bottom: 1px solid $mercury;
-    cursor: pointer;
-
-    &:hover {
-      background-color: $Grey-000;
-    }
-  }
-
-  &__balance {
-    display: flex;
-    flex-direction: column;
-    margin-left: 15px;
-    flex: 1;
-    min-width: 0;
-  }
-
   &__chevron-right {
     color: $Grey-500;
   }
+
+  .list-item__subheading {
+    margin-top: 6px;
+    font-size: 14px;
+  }
 }

ui/app/components/app/asset-list/asset-list.js

@@ -6,11 +6,11 @@ import AddTokenButton from '../add-token-button'
 import TokenList from '../token-list'
 import { ADD_TOKEN_ROUTE } from '../../../helpers/constants/routes'
 import AssetListItem from '../asset-list-item'
-import CurrencyDisplay from '../../ui/currency-display'
 import { PRIMARY, SECONDARY } from '../../../helpers/constants/common'
 import { useMetricEvent } from '../../../hooks/useMetricEvent'
 import { useUserPreferencedCurrency } from '../../../hooks/useUserPreferencedCurrency'
 import { getCurrentAccountWithSendEtherInfo, getNativeCurrency, getShouldShowFiat } from '../../../selectors'
+import { useCurrencyDisplay } from '../../../hooks/useCurrencyDisplay'
 
 const AssetList = ({ onClickAsset }) => {
   const history = useHistory()
@@ -41,29 +41,24 @@ const AssetList = ({ onClickAsset }) => {
     numberOfDecimals: secondaryNumberOfDecimals,
   } = useUserPreferencedCurrency(SECONDARY, { ethNumberOfDecimals: 4 })
 
+  const [primaryCurrencyDisplay] = useCurrencyDisplay(
+    selectedAccountBalance,
+    { numberOfDecimals: primaryNumberOfDecimals, currency: primaryCurrency }
+  )
+
+  const [secondaryCurrencyDisplay] = useCurrencyDisplay(
+    selectedAccountBalance,
+    { numberOfDecimals: secondaryNumberOfDecimals, currency: secondaryCurrency }
+  )
+
   return (
     <>
       <AssetListItem
         onClick={() => onClickAsset(nativeCurrency)}
         data-testid="wallet-balance"
-      >
+        primary={primaryCurrencyDisplay}
-        <CurrencyDisplay
+        secondary={showFiat && secondaryCurrencyDisplay}
-          className="asset-list__primary-amount"
-          currency={primaryCurrency}
-          numberOfDecimals={primaryNumberOfDecimals}
-          value={selectedAccountBalance}
       />
-        {
-          showFiat && (
-            <CurrencyDisplay
-              className="asset-list__secondary-amount"
-              currency={secondaryCurrency}
-              numberOfDecimals={secondaryNumberOfDecimals}
-              value={selectedAccountBalance}
-            />
-          )
-        }
-      </AssetListItem>
       <TokenList
         onTokenClick={(tokenAddress) => {
           onClickAsset(tokenAddress)

ui/app/components/app/asset-list/asset-list.scss

@@ -1,13 +0,0 @@
-.asset-list {
-  &__primary-amount {
-    color: $Black-100;
-    font-size: 16px;
-    height: 16px;
-  }
-
-  &__secondary-amount {
-    color: $Grey-500;
-    margin-top: 6px;
-    font-size: 14px;
-  }
-}

ui/app/components/app/index.scss

@@ -6,8 +6,6 @@
 
 @import 'app-header/index';
 
-@import 'asset-list/asset-list';
-
 @import 'asset-list-item/asset-list-item';
 
 @import '../ui/breadcrumbs/index';

ui/app/components/app/token-cell/index.js

@@ -1 +1 @@
-export { default } from './token-cell.container'
+export { default } from './token-cell'

ui/app/components/app/token-cell/token-cell.component.js

@@ -1,111 +0,0 @@
-import classnames from 'classnames'
-import PropTypes from 'prop-types'
-import React, { Component } from 'react'
-import { conversionUtil, multiplyCurrencies } from '../../../helpers/utils/conversion-util'
-import Tooltip from '../../ui/tooltip-v2'
-import { I18nContext } from '../../../contexts/i18n'
-import AssetListItem from '../asset-list-item'
-
-export default class TokenCell extends Component {
-  static contextType = I18nContext
-
-  static propTypes = {
-    address: PropTypes.string,
-    outdatedBalance: PropTypes.bool,
-    symbol: PropTypes.string,
-    string: PropTypes.string,
-    contractExchangeRates: PropTypes.object,
-    conversionRate: PropTypes.number,
-    currentCurrency: PropTypes.string,
-    image: PropTypes.string,
-    onClick: PropTypes.func.isRequired,
-    userAddress: PropTypes.string.isRequired,
-  }
-
-  static defaultProps = {
-    outdatedBalance: false,
-  }
-
-  render () {
-    const t = this.context
-    const {
-      address,
-      symbol,
-      string,
-      contractExchangeRates,
-      conversionRate,
-      onClick,
-      currentCurrency,
-      image,
-      outdatedBalance,
-      userAddress,
-    } = this.props
-    let currentTokenToFiatRate
-    let currentTokenInFiat
-    let formattedFiat = ''
-
-    if (contractExchangeRates[address]) {
-      currentTokenToFiatRate = multiplyCurrencies(
-        contractExchangeRates[address],
-        conversionRate
-      )
-      currentTokenInFiat = conversionUtil(string, {
-        fromNumericBase: 'dec',
-        fromCurrency: symbol,
-        toCurrency: currentCurrency.toUpperCase(),
-        numberOfDecimals: 2,
-        conversionRate: currentTokenToFiatRate,
-      })
-      formattedFiat = currentTokenInFiat.toString() === '0'
-        ? ''
-        : `${currentTokenInFiat} ${currentCurrency.toUpperCase()}`
-    }
-
-    const showFiat = Boolean(currentTokenInFiat) && currentCurrency.toUpperCase() !== symbol
-
-    const warning = outdatedBalance
-      ? (
-        <Tooltip
-          interactive
-          position="bottom"
-          html={(
-            <div className="token-cell__outdated-tooltip">
-              { t('troubleTokenBalances') }
-              <a
-                href={`https://ethplorer.io/address/${userAddress}`}
-                rel="noopener noreferrer"
-                target="_blank"
-                style={{ color: '#F7861C' }}
-              >
-                { t('here') }
-              </a>
-            </div>
-          )}
-        >
-          <i className={classnames(['fa', 'fa-exclamation-circle', 'token-cell__outdated-icon'])} />
-        </Tooltip>
-      )
-      : null
-
-    return (
-      <AssetListItem
-        className={classnames('token-cell', { 'token-cell--outdated': outdatedBalance })}
-        iconClassName="token-cell__icon"
-        onClick={onClick.bind(null, address)}
-        tokenAddress={address}
-        tokenImage={image}
-        warning={warning}
-      >
-        <div className="token-cell__balance-wrapper">
-          <div className="token-cell__token-balance">{string || 0}</div>
-          <div className="token-cell__token-symbol">{symbol}</div>
-          {showFiat && (
-            <div className="token-cell__fiat-amount">
-              {formattedFiat}
-            </div>
-          )}
-        </div>
-      </AssetListItem>
-    )
-  }
-}

ui/app/components/app/token-cell/token-cell.container.js

@@ -1,14 +0,0 @@
-import { connect } from 'react-redux'
-import TokenCell from './token-cell.component'
-import { getSelectedAddress } from '../../../selectors'
-
-function mapStateToProps (state) {
-  return {
-    contractExchangeRates: state.metamask.contractExchangeRates,
-    conversionRate: state.metamask.conversionRate,
-    currentCurrency: state.metamask.currentCurrency,
-    userAddress: getSelectedAddress(state),
-  }
-}
-
-export default connect(mapStateToProps)(TokenCell)

ui/app/components/app/token-cell/token-cell.js

@@ -0,0 +1,98 @@
+import classnames from 'classnames'
+import PropTypes from 'prop-types'
+import React from 'react'
+import { conversionUtil, multiplyCurrencies } from '../../../helpers/utils/conversion-util'
+import Tooltip from '../../ui/tooltip-v2'
+import AssetListItem from '../asset-list-item'
+import { useSelector } from 'react-redux'
+import { getTokenExchangeRates, getConversionRate, getCurrentCurrency, getSelectedAddress } from '../../../selectors'
+import { useI18nContext } from '../../../hooks/useI18nContext'
+import InfoIcon from '../../ui/icon/info-icon.component'
+import { formatCurrency } from '../../../helpers/utils/confirm-tx.util'
+
+export default function TokenCell ({ address, outdatedBalance, symbol, string, image, onClick }) {
+  const contractExchangeRates = useSelector(getTokenExchangeRates)
+  const conversionRate = useSelector(getConversionRate)
+  const currentCurrency = useSelector(getCurrentCurrency)
+  const userAddress = useSelector(getSelectedAddress)
+  const t = useI18nContext()
+
+  let currentTokenToFiatRate
+  let currentTokenInFiat
+  let formattedFiat = ''
+
+
+  // if the conversionRate is 0 eg: currently unknown
+  // or the contract exchange rate is currently unknown
+  // the effective currentTokenToFiatRate is 0 and erroneous.
+  // Skipping this entire block will result in fiat not being
+  // shown to the user, instead of a fiat value of 0 for a non-zero
+  // token amount.
+  if (conversionRate > 0 && contractExchangeRates[address]) {
+    currentTokenToFiatRate = multiplyCurrencies(
+      contractExchangeRates[address],
+      conversionRate
+    )
+    currentTokenInFiat = conversionUtil(string, {
+      fromNumericBase: 'dec',
+      fromCurrency: symbol,
+      toCurrency: currentCurrency.toUpperCase(),
+      numberOfDecimals: 2,
+      conversionRate: currentTokenToFiatRate,
+    })
+    formattedFiat = `${formatCurrency(currentTokenInFiat, currentCurrency)} ${currentCurrency.toUpperCase()}`
+  }
+
+  const showFiat = Boolean(currentTokenInFiat) && currentCurrency.toUpperCase() !== symbol
+
+  const warning = outdatedBalance
+    ? (
+      <Tooltip
+        interactive
+        position="bottom"
+        html={(
+          <div className="token-cell__outdated-tooltip">
+            { t('troubleTokenBalances') }
+            <a
+              href={`https://ethplorer.io/address/${userAddress}`}
+              rel="noopener noreferrer"
+              target="_blank"
+              style={{ color: '#F7861C' }}
+            >
+              { t('here') }
+            </a>
+          </div>
+        )}
+      >
+        <InfoIcon severity="warning" />
+      </Tooltip>
+    )
+    : null
+
+  return (
+    <AssetListItem
+      className={classnames('token-cell', { 'token-cell--outdated': outdatedBalance })}
+      iconClassName="token-cell__icon"
+      onClick={onClick.bind(null, address)}
+      tokenAddress={address}
+      tokenImage={image}
+      warning={warning}
+      primary={`${string || 0} ${symbol}`}
+      secondary={showFiat ? formattedFiat : undefined}
+    />
+
+  )
+}
+
+TokenCell.propTypes = {
+  address: PropTypes.string,
+  outdatedBalance: PropTypes.bool,
+  symbol: PropTypes.string,
+  string: PropTypes.string,
+  image: PropTypes.string,
+  onClick: PropTypes.func.isRequired,
+}
+
+TokenCell.defaultProps = {
+  outdatedBalance: false,
+}

ui/app/components/app/token-cell/token-cell.scss

@@ -1,95 +1,5 @@
-$wallet-balance-breakpoint: 890px;
-$wallet-balance-breakpoint-range: "screen and (min-width: #{$break-large}) and (max-width: #{$wallet-balance-breakpoint})";
-
 .token-cell {
-  position: relative;
+  &--outdated .list-item__heading {
-
-  &__token-balance {
-    margin-right: 4px;
-    white-space: nowrap;
-    overflow: hidden;
-    text-overflow: ellipsis;
-    min-width: 0;
-    max-width: 100%;
-  }
-
-  &__token-balance, &__token-symbol {
-    font-size: 16px;
-    flex: 0 0 auto;
-    color: $Black-100;
-  }
-
-  &__fiat-amount {
-    margin-top: 6px;
-    font-size: 14px;
-    width: 100%;
-    text-transform: uppercase;
     color: $Grey-500;
   }
-
-  &--outdated &__icon {
-    opacity: 0.5
-  }
-  &--outdated &__balance-wrapper {
-    opacity: 0.5
-  }
-
-  &__balance-wrapper {
-    flex: 1;
-    flex-flow: row wrap;
-    display: flex;
-    min-width: 0;
-  }
-
-  &__outdated-icon {
-    color: $warning-yellow;
-    display: block;
-    padding: 0 10px;
-  }
-
-  &__outdated-tooltip {
-    width: 260px;
-  }
-}
-
-.token-menu-dropdown {
-  width: 80%;
-  position: absolute;
-  top: 52px;
-  right: 25px;
-  z-index: 2000;
-
-  @media #{$wallet-balance-breakpoint-range} {
-    right: 18px;
-  }
-
-  &__close-area {
-    position: fixed;
-    top: 0;
-    left: 0;
-    z-index: 2100;
-    width: 100%;
-    height: 100%;
-    cursor: default;
-  }
-
-  &__container {
-    padding: 16px;
-    z-index: 2200;
-    position: relative;
-  }
-
-  &__options {
-    display: flex;
-    flex-direction: column;
-    justify-content: center;
-  }
-
-  &__option {
-    color: $white;
-    font-family: Roboto;
-    font-size: 16px;
-    line-height: 21px;
-    text-align: center;
-  }
 }

ui/app/components/app/token-cell/token-cell.test.js

@@ -59,16 +59,12 @@ describe('Token Cell', function () {
     assert.equal(wrapper.find(Identicon).prop('image'), './test-image')
   })
 
-  it('renders token balance', function () {
+  it('renders token balance and symbol', function () {
-    assert.equal(wrapper.find('.token-cell__token-balance').text(), '5.000')
+    assert.equal(wrapper.find('.list-item__heading').text(), '5.000 TEST ')
-  })
-
-  it('renders token symbol', function () {
-    assert.equal(wrapper.find('.token-cell__token-symbol').text(), 'TEST')
   })
 
   it('renders converted fiat amount', function () {
-    assert.equal(wrapper.find('.token-cell__fiat-amount').text(), '0.52 USD')
+    assert.equal(wrapper.find('.list-item__subheading').text(), '$0.52 USD')
   })
 
   it('calls onClick when clicked', function () {

ui/app/components/ui/list-item/index.scss

@@ -18,7 +18,7 @@
   }
 
   &__col {
-    align-self: flex-start;
+    align-self: center;
     &-main {
       flex-grow: 1;
     }
@@ -32,7 +32,6 @@
     &-wrap {
       display: inline-block;
       position: absolute;
-      top: 2px;
       width: 16px;
       height: 16px;
       margin-left: 8px;

ui/app/components/ui/list-item/list-item.component.js

@@ -2,11 +2,22 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import classnames from 'classnames'
 
-export default function ListItem ({ title, subtitle, onClick, subtitleStatus, children, titleIcon, icon, rightContent, className }) {
+export default function ListItem ({
+  title,
+  subtitle,
+  onClick,
+  subtitleStatus,
+  children,
+  titleIcon,
+  icon,
+  rightContent,
+  className,
+  'data-testid': dataTestId,
+}) {
   const primaryClassName = classnames('list-item', className)
 
   return (
-    <div className={primaryClassName} onClick={onClick}>
+    <div className={primaryClassName} onClick={onClick} data-testid={dataTestId}>
       {icon && (
         <div className="list-item__col list-item__icon">
           {icon}
@@ -48,4 +59,5 @@ ListItem.propTypes = {
   rightContent: PropTypes.node,
   className: PropTypes.string,
   onClick: PropTypes.func,
+  'data-testid': PropTypes.string,
 }