In some other places, we intentionally put the symbol and the value in separate elements so that we can use CSS to ellipse long values without obscuring the symbol. That seems like it would be useful here too 🤔 But that said I can't see any evidence of such a strategy being used here.

Added back!

NIT: the positional change here is unnecessary and breaks alphabetical order.

[req]

instead of primary/primarySymbol etc

what if we had:
primaryAsset.amount and primaryAsset.symbol and primaryAsset.isFiat ( also primaryAsset.decimals and primaryAsset.address and primaryAsset.image )?
similarily
secondaryAsset.amount amd secondaryAsset.symbol

I think this could go a long way in reducing confusion in this component and make clearer what primary/secondary means. I still don't like these words as a data descriptor. They are being described as how they usually appear in our UI which leads to the confusion that needs to have flags for reversing their meaning. That's a conversation for another PR I think 😬

actually after reviewing this bit:

  const tokenSymbol = useMemo(
    () => (primaryIsFiat ? secondarySymbol : primarySymbol),
    [primaryIsFiat, secondarySymbol, primarySymbol],
  );

I'm more confused.

the asset row exists to display a specific asset, the user's preferences do not change that. primary in this case is meant to describe the larger, more dominant value whereas secondary is the one under it that is smaller and supports the former.

I think we should go with something like this:

asset: PropTypes.shape({
  amount: PropTypes.string.isRequired,
  symbol: PropTypes.string.isRequired,
  decimals: PropTypes.number,
  address: PropTypes.string,
  image: PropTypes.string,
}).isRequired,
conversionAsset: PropTypes.shape({
  amount: PropTypes.string.isRequired,
  symbol: PropTypes.string.isRequired,
}),
conversionIsPrimary: propTypes.bool,

we can then always rely on the distinction that asset is the asset we are rendering. conversion is clearer as well than secondary, especially with the combination of conversionIsPrimary. We also do not extend the problem of treating the "secondary" asset as fiat when it could be other crypto. I probably haven't thought this all the way through, so feel free to mutate it to make sense but I think this is much easier to reason about when trying to support this component moving forward.

Also related, when conversionRate is undefined (hardcoded in getConversionRate):

Re the last comment, the conversionRate should be null when not available? and I'm not seeing that behavior where some tokens are USD and others are with their symbols...?

Regarding the shape you suggest for primary/secondary -> asset/conversionAsset, I agree it is an improvement. I'll move forward with that for now.

ah sorry, you have to set "primary" currency as "Fiat" in settings to see this. I did update the getConversionRate selector to return null instead of undefined and same result.

I think your intention may have been to utilize the token-value styles to ellipse long text here. If that's the case you need to add the class to those styles.

can this use the getShouldShowFiat selector now?

yessir!

This file throws an error when getConversionRate is hardwired to return undefined, because it doesn't check if this is available before trying to compute values.

left: this PR, "Fiat" selected for primary, getConversionRate set to return undefined.

My assumption would be that, when the conversionRate is undefined, that the original UI (right) is shown but without the $0.00?

this boolean needs to be updated to take into account the possibility of conversionRate not being present.

Something in the swaps changes has caused the middle row to never fill in fee estimate regardless of conversionRate being defined or not: