Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

style(frontend): fixes display of token balances #3234

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+27 −13
Open

style(frontend): fixes display of token balances #3234

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dcc4467
fixes display of token balances
BonomoAlessandro Oct 30, 2024
2a2dd2c
undo TokenBalance.svelte changes
BonomoAlessandro Oct 30, 2024
9cad2b9
fixes token balance and usd balance display
BonomoAlessandro Oct 30, 2024
7a784b3
adds customizable nullish balance message to the TokenExchangeBalance…
BonomoAlessandro Oct 30, 2024
d98cfd7
uses i18n for texts
BonomoAlessandro Nov 6, 2024
219d74a
Merge branch 'main' into style(frontend)/fix-token-balances
BonomoAlessandro Nov 6, 2024
69df53e
updates i18n texts
BonomoAlessandro Nov 6, 2024
6ac9317
🤖 Updated i18n files
github-actions[bot] Nov 6, 2024
922e314
removes used default value of variable
BonomoAlessandro Nov 13, 2024
96b5b2c
Merge branch 'main' into style(frontend)/fix-token-balances
BonomoAlessandro Nov 13, 2024
9a984ed
updates styling
BonomoAlessandro Nov 15, 2024
aa5d2de
Merge branch 'main' into style(frontend)/fix-token-balances
BonomoAlessandro Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/frontend/src/lib/components/hero/Balance.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
</output>

<span class="text-xl font-bold opacity-50">
<TokenExchangeBalance balance={token?.balance} usdBalance={token?.usdBalance} />
<TokenExchangeBalance
balance={token?.balance}
usdBalance={token?.usdBalance}
nullishBalanceMessage="$ value is not available"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i18n ?

/>
</span>
</span>
6 changes: 1 addition & 5 deletions src/frontend/src/lib/components/tokens/TokenBalance.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<script lang="ts">
import { nonNullish } from '@dfinity/utils';
import TokenBalanceSkeleton from '$lib/components/tokens/TokenBalanceSkeleton.svelte';
import { ZERO } from '$lib/constants/app.constants';
import { TOKEN_BALANCE } from '$lib/constants/test-ids.constants';
import type { CardData } from '$lib/types/token-card';
import { formatToken } from '$lib/utils/format.utils';
Expand All @@ -17,10 +16,7 @@
unitName: data.decimals
})}
{:else}
{formatToken({
value: ZERO,
unitName: data.decimals
}).replace('0', '-')}
<span>n/a</span>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

{/if}
</output>
</TokenBalanceSkeleton>
10 changes: 4 additions & 6 deletions src/frontend/src/lib/components/tokens/TokenExchangeBalance.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
export let balance: TokenFinancialData['balance'];
export let usdBalance: TokenFinancialData['usdBalance'];
export let nullishBalanceMessage = '-';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a prop? I think you can use directly inside the component since that is its usage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not convinced about this, i would prefer to default is as undefined and the use coalescing to overwrite it with '-'

</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>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need the condition here anymore, right? it will always be true

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but animate-pulse={isNullish(balance)} will always be true, since it's inside if isNullish(balance)}

{:else}
<span class:animate-pulse={isNullish(balance)}
>{formatUSD({
value: 0,
options: { minFraction: 0, maxFraction: 0 }
}).replace('0', '-')}
</span>
<span class:animate-pulse={isNullish(balance)}>n/a</span>
{/if}
</output>