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

Add "Token Holders" tab to Token Dashboard #635

Merged
merged 7 commits into from
Jul 3, 2023

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Jun 30, 2023

This is built on top of #634, and implements the last missing tab, "Token Holders".

Design is here.

@github-actions
Copy link

github-actions bot commented Jun 30, 2023

Deployed to Cloudflare Pages

Latest commit: f347efadd96186e757ef3ca26bbbdb7706fe47cb
Status:✅ Deploy successful!
Preview URL: https://a6899541.oasis-explorer.pages.dev

@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch 2 times, most recently from 46bf6f6 to e81c7e1 Compare June 30, 2023 16:39
@csillag csillag marked this pull request as ready for review June 30, 2023 16:39
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch 2 times, most recently from 950f482 to 3f1042d Compare June 30, 2023 16:45
@csillag csillag requested review from lukaw3d and buberdds June 30, 2023 16:48
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch 5 times, most recently from 4840437 to b3bfc8a Compare July 3, 2023 09:10
},
{
key: 'quantity',
content: t('tokens.totalSupplyValue', { value: fromBaseUnits(holder.balance, decimals) }),
Copy link
Contributor

@buberdds buberdds Jul 3, 2023

Choose a reason for hiding this comment

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

Shouldn't we use RoundedBalance here for Quantity? Not sure about Percentage. for example
http://localhost:1234/mainnet/sapphire/token/0x4344919960B0196c512807eE2f3922302fFFE542/holders#holders

-- | --
1 | 0x3BA9F711C9808902f6a200d3715ba8a7e4D8b63f | 100 | 100.0000%
2 | 0x00000000000000000000000000000000000000ff | 0 | 0.0000%

we show 0 in the second row

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't we use RoundedBalance here for Quantity?

OK done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about Percentage. for example http://localhost:1234/mainnet/sapphire/token/0x4344919960B0196c512807eE2f3922302fFFE542/holders#holders

-- | --
1 | 0x3BA9F711C9808902f6a200d3715ba8a7e4D8b63f | 100 | 100.0000%
2 | 0x00000000000000000000000000000000000000ff | 0 | 0.0000%

we show 0 in the second row

I don't see the issue here. The percentage must obviously be a rounded value, so what else would anyone expect?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what the issue is here; I think I agree here with @csillag

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, so I think we are good here.

Copy link
Member

Choose a reason for hiding this comment

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

I would expect
da222769 oasis-explorer pages dev_mainnet_sapphire_token_0x4344919960B0196c512807eE2f3922302fFFE542_holders (1)
not
da222769 oasis-explorer pages dev_mainnet_sapphire_token_0x4344919960B0196c512807eE2f3922302fFFE542_holders

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't we use RoundedBalance here for Quantity?

Actually, the current implementation of RoundedBalance is not very good with bigger numbers.
I think vanilla i18n is actually better, for now..

vanilla i18next RoundedBalance
kép kép

src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch from 5751de4 to 949fd67 Compare July 3, 2023 11:42
@csillag csillag requested a review from buberdds July 3, 2023 12:35
src/app/components/Tokens/TokenHolders.tsx Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Outdated Show resolved Hide resolved
src/app/pages/TokenDashboardPage/TokenHoldersCard.tsx Outdated Show resolved Hide resolved
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch 7 times, most recently from aa85b8e to 2bc8ab3 Compare July 3, 2023 15:32
src/oasis-nexus/api.ts Outdated Show resolved Hide resolved
src/app/components/Tokens/TokenHolders.tsx Show resolved Hide resolved
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch from 2bc8ab3 to e33809a Compare July 3, 2023 18:51
@csillag csillag force-pushed the csillag/token-dashboard-last-tab branch from e33809a to f347efa Compare July 3, 2023 18:53
@csillag csillag merged commit df99465 into master Jul 3, 2023
7 checks passed
@csillag csillag deleted the csillag/token-dashboard-last-tab branch July 3, 2023 18:56
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.

None yet

4 participants