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

Token allocation chart #914

Merged
merged 13 commits into from
Sep 8, 2023
Merged

Token allocation chart #914

merged 13 commits into from
Sep 8, 2023

Conversation

bobo-k2
Copy link
Contributor

@bobo-k2 bobo-k2 commented Aug 31, 2023

Pull Request Summary

image

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tested on mobile devices

This pull request makes the following changes:

Adds

  • (ex: Add feature A)

Fixes

  • (ex: Fix validation function)

Changes

  • (ex: Change document B)

To-dos

*Feel free to remove this section if it's not applicable

  • (ex: add user list)

@github-actions
Copy link

github-actions bot commented Aug 31, 2023

Visit the preview URL for this PR (updated for commit 10b117d):

https://astar-apps--pr914-feat-token-chart-g9s5kgen.web.app

(expires Thu, 14 Sep 2023 12:16:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db

@Kahonnohak
Copy link
Contributor

Looks good!

So the main issue here is the Others still including some tokens from circulation supply. Please check

  • According to our current Portal, Others should be 37% and Circulation should be 63%, those need to be dynamic.
  • In side of TVL it is nice if we know how much are unbonding but not "must"
Screenshot 2023-09-01 at 09 09 38 Screenshot 2023-09-01 at 09 12 03

@bobo-k2
Copy link
Contributor Author

bobo-k2 commented Sep 1, 2023

https://astar-apps--pr914-feat-token-chart-g9s5kgen.web.app

Looks good!

So the main issue here is the Others still including some tokens from circulation supply. Please check

  • According to our current Portal, Others should be 37% and Circulation should be 63%, those need to be dynamic.
  • In side of TVL it is nice if we know how much are unbonding but not "must"

Yes, I am aware of that, but the issue happens because I am missing data for some token allocations. In reality, we have the following situation
37% - others
44% - tvl
4% - treasury
15% - unknown

I can add one more section to the chart for this unknown allocations, but not sure how to call it

@Kahonnohak
Copy link
Contributor

so 15% must be the tokens on the market, so please use blue gradients that is very clear for people to see one in circulation, and the other is not.

Thats why I wanted to use the word "locked" for Not Circulating supply instead of Other, so we can use the Other for some tokens that is in the wallets. Let me confirm the name with Maarten once the chart allocation is fixed.

@bobo-k2
Copy link
Contributor Author

bobo-k2 commented Sep 1, 2023

Added 4th segment. Also added two decimal point to percentage, otherwise percentage rounding make such mistake that at the end percentages sum up to 101%

image

@Kahonnohak
Copy link
Contributor

Right!

1, Wording
In the pie chart
Not Circulation Supply -> Others
Not categorised (Other currently) -> Circulating
TVL-> Staking

The out side of the chart, right bottom
Circulating
Others

I added (s) to Other.

That should be aligned and okay!

2, Could you also adjust padding a bit, I could not see the exact numbers but just a little..

Screenshot 2023-09-01 at 16 05 13

@bobo-k2 bobo-k2 marked this pull request as ready for review September 5, 2023 09:42
@Kahonnohak
Copy link
Contributor

Bobo, sorry, I was not looking at the Light theme.

Could you change the Other colour in the chart from Navy to Grey001 F7F7F8. It might be too light but I could not check the change in the inspect🙇

Copy link
Contributor

@Kahonnohak Kahonnohak left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@gluneau gluneau left a comment

Choose a reason for hiding this comment

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

LGTM, it works.

@bobo-k2 bobo-k2 merged commit 64e50e7 into main Sep 8, 2023
6 checks passed
@bobo-k2 bobo-k2 deleted the feat/token-chart branch September 8, 2023 06:37
bobo-k2 added a commit that referenced this pull request Sep 13, 2023
* Initial chart implementation

* Chart control styling and improvements

* Light theme support

* Styling improvement

* Translations

* Added 4th segment

* Code review updates

* Don't display 0 values

* e2e test engine improvements

* Test fixes

* XCM003 fix

* Improved Metamask tests
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