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.

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

Round token supply number in token list #703

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Round token supply number in token list #703

merged 1 commit into from
Jul 11, 2023

Conversation

buberdds
Copy link
Contributor

@buberdds buberdds commented Jul 10, 2023

Token supply has too many decimal points

@github-actions
Copy link

github-actions bot commented Jul 10, 2023

Deployed to Cloudflare Pages

Latest commit: d4a3e50ac0b2c52e217de0bfab3e3691bf5c6669
Status:✅ Deploy successful!
Preview URL: https://45a5399e.oasis-explorer.pages.dev

.changelog/702.trivial.md Outdated Show resolved Hide resolved
.changelog/702.trivial.md Outdated Show resolved Hide resolved
@@ -129,7 +130,7 @@ export const TokenList = (props: TokensProps) => {
align: TableCellAlign.Right,
},
{
content: token.total_supply,
content: <RoundedBalance value={token.total_supply} ticker={token.symbol} />,
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I prefer this way of formatting the supply: https://github.com/oasisprotocol/explorer/blob/master/src/locales/en/translation.json#L215

With rounded balance:
image

Via i18next:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

But I also understand that it is not great with decimals.
I think we need a way that satisfies both criteria.

Can we have the same grouping in RoundedBalance?

(Also, maybe we could consider to resurrect #333?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at task for now we care about decimals only. Prob Don will revisit it all later. i18n does not match logic in RoundedDecimals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think we are good here. As discussed during meeting, RoundedBalance logic will be adjusted in another PR.

Copy link
Contributor

@csillag csillag left a comment

Choose a reason for hiding this comment

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

Reviewed. LGTM

@buberdds buberdds merged commit c35d737 into master Jul 11, 2023
@buberdds buberdds deleted the mz/sup branch July 11, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants