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 network tables for Hubble Stats #1452

Merged
merged 16 commits into from
Jul 26, 2023

Conversation

vutuanlinh2k2
Copy link
Contributor

@vutuanlinh2k2 vutuanlinh2k2 commented Jul 19, 2023

Summary of changes

Provide a detailed description of proposed changes.

  • Add Network Pool Table in Hubble Stats
  • Add Network Token Table in Hubble Stats
  • Add Wrapping Fees row in Pool Metadata table in Hubble Stats

Proposed area of change

Put an x in the boxes that apply.

  • apps/bridge-dapp
  • apps/hubble-stats
  • apps/stats-dapp
  • apps/webbsite
  • apps/faucet
  • apps/tangle-website
  • libs/webb-ui-components

Reference issue to close (if applicable)

Specify any issues that can be closed from these changes (e.g. Closes #233).

Screen Recording (with dummy data)

If possible provide a screen recording of proposed change.

Screen.Recording.2023-07-24.at.20.04.24.mov

Code Checklist

Please be sure to add .stories documentation if any additions are made to libs/webb-ui-components.

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit c61029e1740d26e019a34abad883f869cf234dc1
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64b800d44900f028ab3b0d60
😎 Deploy Preview https://64b800d44900f028ab3b0d60--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@monaiuu
Copy link
Contributor

monaiuu commented Jul 19, 2023

Looks good. Quick heads up there may be updates required to this table based on recent discussions in displaying pool & token data. Will create a separate issue to address the updates.

Comment on lines 10 to 14
const [chains, setChains] = useState<number[]>([]);
const [tvlData, setTvlData] = useState<TokenCompositionType[]>([]);
const [volumeData, setVolumeData] = useState<TokenCompositionType[]>([]);
const [depositsData, setDepositsData] = useState<TokenCompositionType[]>([]);
const [feesData, setFeesData] = useState<TokenCompositionType[]>([]);
Copy link
Member

Choose a reason for hiding this comment

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

It would be a good idea to transform this component into a server component. You could do this by eliminating all instances of useState. To emulate data fetching, you can create a data fetching function outside of the component, which returns dummy data.

Next, transform this component into an asynchronous one and call the newly created data fetching function to obtain data. Once you have the data, you can pass it into the child components for rendering. This way, you'll maintain a smoother and more efficient component structure.

Copy link
Member

@AtelyPham AtelyPham left a comment

Choose a reason for hiding this comment

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

Left some comments.

@vutuanlinh2k2 vutuanlinh2k2 added the wip 🚧 Work in-progress label Jul 20, 2023
@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit 6fb3a74442a151616ca529f2aaa64a36ef7bdbfc
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64b93cbd25c4c10a07bd0e20
😎 Deploy Preview https://64b93cbd25c4c10a07bd0e20--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@vutuanlinh2k2 vutuanlinh2k2 changed the title Add network table for Hubble Stats Add network tables for Hubble Stats Jul 24, 2023
@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit 76939c3d18c9e05e5e2d4cacac41c36c899d20a4
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64be87449d28120092e696a1
😎 Deploy Preview https://64be87449d28120092e696a1--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@vutuanlinh2k2 vutuanlinh2k2 added needs review 👓 Pull request needs a review and removed wip 🚧 Work in-progress labels Jul 24, 2023
Copy link
Contributor

@monaiuu monaiuu left a comment

Choose a reason for hiding this comment

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

LGTM!

Quick question: could we implement a sorting mechanism for the order in which we display the chains within the table? One idea is that we could display them alphabetically (i.e., Arbitrum Goerli, Avalanche Fuji, Ethereum Goerli, Ethereum Sepolia.... -> Scroll Alpha etc.)

Open to other ideas too, let me know your thoughts.

@vutuanlinh2k2
Copy link
Contributor Author

LGTM!

Quick question: could we implement a sorting mechanism for the order in which we display the chains within the table? One idea is that we could display them alphabetically (i.e., Arbitrum Goerli, Avalanche Fuji, Ethereum Goerli, Ethereum Sepolia.... -> Scroll Alpha etc.)

Open to other ideas too, let me know your thoughts.

I think this can be implemented, I will try and update soon!

@monaiuu
Copy link
Contributor

monaiuu commented Jul 24, 2023

LGTM!
Quick question: could we implement a sorting mechanism for the order in which we display the chains within the table? One idea is that we could display them alphabetically (i.e., Arbitrum Goerli, Avalanche Fuji, Ethereum Goerli, Ethereum Sepolia.... -> Scroll Alpha etc.)
Open to other ideas too, let me know your thoughts.

I think this can be implemented, I will try and update soon!

Sounds good 💪 It'd be nice to have consistent order when users navigate between various pools.

@vutuanlinh2k2
Copy link
Contributor Author

@monaiuu I have added new code to sort the chains alphabetically!

Screenshot 2023-07-25 at 09 53 10

@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @AtelyPham

Name Link
🔨 Latest commit 7360a50f9c7c2784d8208022aaa5b318629a9925
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64bf9aed7668bf4c92b5cff9
😎 Deploy Preview https://64bf9aed7668bf4c92b5cff9--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@monaiuu
Copy link
Contributor

monaiuu commented Jul 25, 2023

@monaiuu I have added new code to sort the chains alphabetically!

Screenshot 2023-07-25 at 09 53 10

Hmm, if following the logic of the first letter, I think the order would be as follow:

Arbitrum Goerli
Avalanche Fuji
Ethereum Goerli
Ethereum Sepolia
Moonbase Alpha
Optimism Goerli
Polygon Mumbai
Scroll Alpha

Otherwise looks good! We want to make sure the logic is valid and can be scaled across various pools. Let's get it merged after fixing this small ordering issue.

@monaiuu
Copy link
Contributor

monaiuu commented Jul 25, 2023

@monaiuu I have added new code to sort the chains alphabetically!
Screenshot 2023-07-25 at 09 53 10

Hmm, if following the logic of the first letter, I think the order would be as follow:

Arbitrum Goerli Avalanche Fuji Ethereum Goerli Ethereum Sepolia Moonbase Alpha Optimism Goerli Polygon Mumbai Scroll Alpha

Otherwise looks good! We want to make sure the logic is valid and can be scaled across various pools. Let's get it merged after fixing this small ordering issue.

@AtelyPham @vutuanlinh2k2 If this takes too much effort to do, we can overlook it for now and revisit it later on. Otherwise everything looks good and I think it is ready to merge.

@vutuanlinh2k2
Copy link
Contributor Author

@monaiuu I have added new code to sort the chains alphabetically!
Screenshot 2023-07-25 at 09 53 10

Hmm, if following the logic of the first letter, I think the order would be as follow:
Arbitrum Goerli Avalanche Fuji Ethereum Goerli Ethereum Sepolia Moonbase Alpha Optimism Goerli Polygon Mumbai Scroll Alpha
Otherwise looks good! We want to make sure the logic is valid and can be scaled across various pools. Let's get it merged after fixing this small ordering issue.

@AtelyPham @vutuanlinh2k2 If this takes too much effort to do, we can overlook it for now and revisit it later on. Otherwise everything looks good and I think it is ready to merge.

@monaiuu I made a small adjustment to the code so the chain with the same group will be next to each other. @AtelyPham please review the code one last time and merge!

Screenshot 2023-07-26 at 09 50 09

@vutuanlinh2k2 vutuanlinh2k2 force-pushed the linh/hubble-stats-network-table branch from 7360a50 to bc7b6d2 Compare July 26, 2023 02:51
@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @AtelyPham

Name Link
🔨 Latest commit b8e0484b7eb479ee3d57f394bf57cefab14ab6fd
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64c0a94c7dfcba240f6ef9f3
😎 Deploy Preview https://64c0a94c7dfcba240f6ef9f3--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@AtelyPham AtelyPham left a comment

Choose a reason for hiding this comment

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

I left some comments.

<div className="w-full text-center">
<ChainChip
chainName={chainsConfig[chainId].name}
chainType={chainsConfig[chainId].group as ChainGroup}
Copy link
Member

Choose a reason for hiding this comment

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

I believe it would be beneficial if we consider changing the type of the group property to ChainGroup. This is due to the observation that all the chains in the chainsConfig seem to have a predefined value for the group property. Hence, the need for ChainGroup | undefined may not be necessary. What do you think?

apps/hubble-stats/components/NetworkPoolTable/types.ts Outdated Show resolved Hide resolved
apps/hubble-stats/components/NetworkPoolTable/types.ts Outdated Show resolved Hide resolved
apps/hubble-stats/components/NetworkTokenTable/types.ts Outdated Show resolved Hide resolved
@github-actions
Copy link

Deploy Preview for webb hubble statistic is ready! Thanks for the contribution @vutuanlinh2k2

Name Link
🔨 Latest commit 96628f9a07df257b061d51db9e238c2ae074e30c
🔍 Latest deploy log https://app.netlify.com/sites/hubble-stats/deploys/64c0c7eb9ac42732a76ca64c
😎 Deploy Preview https://64c0c7eb9ac42732a76ca64c--hubble-stats.netlify.app

To edit notification comments on pull requests, go to your Netlify site settings.

@AtelyPham AtelyPham self-requested a review July 26, 2023 07:27
@vutuanlinh2k2 vutuanlinh2k2 merged commit 225f168 into develop Jul 26, 2023
7 checks passed
@vutuanlinh2k2 vutuanlinh2k2 deleted the linh/hubble-stats-network-table branch July 26, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review 👓 Pull request needs a review
Projects
None yet
3 participants