-
Notifications
You must be signed in to change notification settings - Fork 70
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
4326-Validators list - Page and widgets #4375
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Do I have to list all the active validators for the net or the ones of the account who is signed in the page? |
All according to the design |
const { api } = useApi() | ||
// const [staking, setStaking] = useState<Staking | any>(null) | ||
// const [validators, setValidators] = useState<ValidatorProps[]>([]) | ||
// const [nominators, setNominators] = useState<NominatorProps[]>([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the commented lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I removed the comment lines
totalStake, | ||
idealStaking, | ||
eraDuration, | ||
activeEra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add return values for "totalRewards" and "lastRewards"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the code, so the "lastRewards" value was implemented.
But I can't find the exact api function to get the value for "total Rewards".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For validator rewards (allocated but unminted) we need to know the first and last era and sum them up from API staking.erasValidatorReward
, unless there's a backend to query totals.
e02fad2
to
7cf3349
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eshark9312 almost there ! I added a few change requests mostly about reducing the amount of rpc queries. Apart from I pushed a small change related to the mocked chain unwrap
so you should probably pull the branch before adding changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a regression on the latest commits. Here's the current Storybook:
https://pioneer-2-storybook-git-fork-eshark9312-valida-b7c579-joystream.vercel.app/?path=/story/pages-validators-validatorlist--statistics
And here's was the one before the last push:
https://pioneer-2-storybook-iz2xsiyh3-joystream.vercel.app/?path=/story/pages-validators-validatorlist--statistics
Also the "100%" style is a bit broken because the 1 overflows on the border.
if (activeValidators && api) | ||
return activeEra ? combineLatest(activeValidators.map((address) => api.query.staking.erasStakers(activeEra.eraIndex, address))): undefined | ||
return ( | ||
activeEra && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last really small detail but this code is mixing an if
statement and a js type coercion/short-circuit evaluation (I'm not sure what to call it actually 😅). To keep the code clearer please chose one of these 2
Either the if
statement:
const stakers = useObservable(() => {
if (activeValidators && api && activeEra)
return combineLatest(activeValidators.map((address) => api.query.staking.erasStakers(activeEra.eraIndex, address)))
...
Or:
const stakers = useObservable(() =>
activeValidators && api && activeEra &&
combineLatest(activeValidators.map((address) => api.query.staking.erasStakers(activeEra.eraIndex, address)))
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have modified the code according to your review so that the storybook seems to be good now.
And in the Piechart, 100%
is never displayed in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
Resolves #4326
Initialized Validators Module with Validators List pages and widgets.