style(frontend): fixes display of token balances #3234
Conversation
BonomoAlessandro
changed the title
|
So, this PR I think is not correct: it was decided that when the USD balance is Same thing for the balance: if the balance is May you please double-check with the requirements? |
|
The token card looks now like this: balance and usdBalance defined: balance null and usdBalance defined: balance defined and usdBalance undefined: balance null and usdBalance undefined: |
|
The Token view looks like this: usdBalance defined: usdBalance undefined: |
|
@AntonioVentilii-DFINITY pls review |
| <TokenExchangeBalance | ||
| balance={token?.balance} | ||
| usdBalance={token?.usdBalance} | ||
| nullishBalanceMessage="$ value is not available" |
| value: ZERO, | ||
| unitName: data.decimals | ||
| }).replace('0', '-')} | ||
| <span>n/a</span> |
| @@ -5,17 +5,15 @@ | |||
|
|
|||
| export let balance: TokenFinancialData['balance']; | |||
| export let usdBalance: TokenFinancialData['usdBalance']; | |||
| export let nullishBalanceMessage = '-'; | |||
There was a problem hiding this comment.
why a prop? I think you can use directly inside the component since that is its usage
There was a problem hiding this comment.
This needs to be a prop because by default, this property will display - when the value is null or undefined. However, for the Hero element, it should display '$ value is not available' instead.
There was a problem hiding this comment.
I am still not convinced about this, i would prefer to default is as undefined and the use coalescing to overwrite it with '-'
# Conflicts: # src/frontend/src/lib/components/hero/Balance.svelte # src/frontend/src/lib/i18n/en.json
|
@AntonioVentilii-DFINITY pls review |
|
@BonomoAlessandro may you please update the description and the screenshots? |
| </script> | ||
|
|
||
| <output class="break-all"> | ||
| {#if nonNullish(balance) && nonNullish(usdBalance)} | ||
| {formatUSD({ value: usdBalance })} | ||
| {:else if isNullish(balance)} | ||
| <span class:animate-pulse={isNullish(balance)}>{nullishBalanceMessage}</span> |
There was a problem hiding this comment.
you don't need the condition here anymore, right? it will always be true
There was a problem hiding this comment.
No this is still needed. The if above checks if the balance AND usdBalance is non nullish. In case that the balance is defined but the usdbalance is nullish this check also failes and the third option will take place.
There was a problem hiding this comment.
but animate-pulse={isNullish(balance)} will always be true, since it's inside if isNullish(balance)}
| @@ -5,17 +5,15 @@ | |||
|
|
|||
| export let balance: TokenFinancialData['balance']; | |||
| export let usdBalance: TokenFinancialData['usdBalance']; | |||
| export let nullishBalanceMessage = '-'; | |||
There was a problem hiding this comment.
I am still not convinced about this, i would prefer to default is as undefined and the use coalescing to overwrite it with '-'
| options: { minFraction: 0, maxFraction: 0 } | ||
| }).replace('0', '-')} | ||
| </span> | ||
| <span class:animate-pulse={isNullish(balance)}>{$i18n.tokens.balance.error.not_applicable}</span |
There was a problem hiding this comment.
personally, I would not like to see a bunch of 'n/a' on the screen... especially in the ICP network, I don't know if it gives a good impression to the users, @StefanBerger-DFINITY
There was a problem hiding this comment.
@AntonioVentilii at the moment it is defined like this. I guess in case of a change it would be solved with another bug ticket.
| value: ZERO, | ||
| unitName: data.decimals | ||
| }).replace('0', '-')} | ||
| <span>{$i18n.tokens.balance.error.not_applicable}</span> |
There was a problem hiding this comment.
here will be the same, but there are less tokens where this is true; maybe here just greyed out?
There was a problem hiding this comment.
same as my comment above
|
@AntonioVentilii Description is updated and the not needed |
Motivation
There should be a consistent way to display the token balance and the usd balance.
Token card:
n/ashould be displayed for the balance and-. for the usd balance.n/a.n/aand the usd balance as-.Token view:
When the usd balance is not defined, the text
$ value is not availableshould be displayed.Changes
Tests
before:
token card
balance and usdBalance defined:
balance null and usdBalance defined:
balance defined and usdBalance undefined:
balance and usdBalance undefined:
token view
usd balance defined:
usd balance undefined:
after:
token card
balance and usdBalance defined:
balance null and usdBalance defined:
balance defined and usdBalance undefined:
balance and usdBalance undefined:
token view
usd balance defined:
usd balance undefined:
