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

feat: Top accounts #841

Merged
merged 14 commits into from
Jul 15, 2024
Merged

feat: Top accounts #841

merged 14 commits into from
Jul 15, 2024

Conversation

janmichek
Copy link
Collaborator

@janmichek janmichek commented Jun 13, 2024 •
edited
Loading

Description

resolves #616

Demo

firefox_5Ru6TMihAK.mp4

Checklist:

@github-actions GitHub Actions
Copy link

Deployed to https://pr-841-aescan.stg.service.aepps.com and https://pr-841-aescan-testnet.stg.service.aepps.com

@Liubov-crypto
Copy link
Collaborator

is this feature only for mainnet accounts?

it might be worth adding a period at the end of the sentence to make it the same as the other sentences:

bal

the rest LGTM.

@janmichek
Copy link
Collaborator Author

is this feature only for mainnet accounts?

No, why? Different networks has different responses.

it might be worth adding a period at the end of the sentence to make it the same as the other sentences:

Sure, fixed. Thanks

Liubov-crypto
Liubov-crypto approved these changes Jun 17, 2024
View reviewed changes
Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

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

LGTM

@janmichek
Copy link
Collaborator Author

@michele-franchi I was thinking about shortening the list to 100 (just print half of it), as the lower half does not have very important entries

@michele-franchi
Copy link
Collaborator

Agree @janmichek, makes sense.

@janmichek
Copy link
Collaborator Author

Agree @janmichek, makes sense.

fixed

michele-franchi
michele-franchi requested changes Jun 18, 2024
View reviewed changes
src/pages/accounts/index.vue Outdated Show resolved Hide resolved
src/utils/hints/topAccountsHints.js Outdated Show resolved Hide resolved
src/utils/adapters.js
rank: index + 1,
account: account.account,
balance: formatAePrice(formatAettosToAe(account.balance)),
percentage: (formatAettosToAe(account.balance) * 100 / distribution).toFixed(4),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this calculated on the circulating supply?
I see the dashboard component uses totalTokenSupply from blockChainStats.js while here it seems distribution is different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are right, fixed

janmichek and others added 3 commits June 18, 2024 14:55
@janmichek @michele-franchi
fix imports
1be0680 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek @michele-franchi
fix wording
92e998b 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek
fix using distribution
637f77e
@janmichek janmichek merged commit bd6cbeb into develop Jul 15, 2024
4 checks passed
@janmichek janmichek deleted the top-accounts branch July 15, 2024 13:45
This was referenced Sep 9, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Liubov-crypto Liubov-crypto Liubov-crypto approved these changes

@michele-franchi michele-franchi Awaiting requested review from michele-franchi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add top accounts page
3 participants
@janmichek @Liubov-crypto @michele-franchi