-
Notifications
You must be signed in to change notification settings - Fork 121
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
#66 Unifying data types and fake data in staking #67
#66 Unifying data types and fake data in staking #67
Conversation
* cleaned up staking related types
* removed empty reducers
I did not add tests for the actions as they are just placeholders, likely when we fetch real data we have to add some logic to the actions. |
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.
Looks good! I only left a couple very minor comments, just cleaning up old comments
myStaking: MyStaking[], | ||
allValidators: Validator[] | ||
): MyValidators[] => { | ||
// try { |
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.
Looks like maybe this try {
was intended to be removed
tableConfigurations={myBalancesConfigurations} | ||
/> | ||
|
||
{/* my validators */} | ||
<Table | ||
title="My Validators" | ||
data={myValidatorData} | ||
// data={myValidatorData} |
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.
Definitely not a big deal, but this comment could be removed.
What does this PR do
Validator
in staking dataFAQ
Q: Why is the data drilled down through a few layers?
A: While it is quite many, I feel that this view is very likely to change, or even be completely reshuffled for desktop/mobile. So this way maintaining several views for this data will be quick to refactor. So I feel that doing any changes in these stateless views will be very cheap.