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

Implement votes tab on account page - Closes #1779 #1822

Merged
merged 31 commits into from Mar 15, 2019

Conversation

Projects
None yet
2 participants
@massao
Copy link
Contributor

commented Mar 15, 2019

What issue have I solved?

#1779

How have I implemented/fixed it?

Implemented Votes tab on another account page, that consumes the votes api to show the data, loading additional data for delegates will be handled in a follow up ticket: #1820

How has this been tested?

Go to /explorer/accountsV2/:address of some account and check if:

  • it has more than 30 it should appear the show more button at the bottom;
  • it has no votes should show a message.

The filter should filter by username:

  • If a username meets the criteria it should show up;
  • if no username meets the criteria, a message shows up.

Review checklist

massao added some commits Mar 12, 2019

Merge branch '1779-implement-votes-tab-on-account-page' of github.com…
…:LiskHQ/lisk-hub into 1779-implement-votes-tab-on-account-page

@massao massao self-assigned this Mar 15, 2019

@massao massao requested a review from osvaldovega Mar 15, 2019

@osvaldovega
Copy link
Contributor

left a comment

great man, looks good, I think just 1 or 2 changes and the rest are details that I don't know if we should discuss in Monday meeting with @slaweet to see what will be the best way to follow related destructuring in the render

@@ -311,6 +313,7 @@
"Password": "Password",
"Paste": "Paste",
"Pending...": "Pending...",
"Percentage of successfully forged blocks of when the delegate should have forged a block of transactions.": "Percentage of successfully forged blocks of when the delegate should have forged a block of transactions.",

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

If I am not wrong this is not properly write, should be without the of between blocks and when, like this.

Percentage of successfully forged blocks of when the delegate should have forged a block of transactions
Percentage of successfully forged blocks when the delegate should have forged a block of transactions

This comment has been minimized.

Copy link
@massao

massao Mar 15, 2019

Author Contributor

Checked all tooltips with marketing team, and updated where necessary


&.isLoading {
& .loadingOverlay {
background: rgba(255, 255, 255, 0.8);

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

maybe you can use a constant in here

This comment has been minimized.

Copy link
@massao

massao Mar 15, 2019

Author Contributor

This one is because it's just for the mask when in the loading state, that we will probably change for the final design.

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

good

align-items: center;
bottom: 0;
background: var(--color-white);
box-shadow: 0 -1px 5px 0 rgba(30, 25, 77, 0.06);

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

is better to use a constant in here

Show resolved Hide resolved src/components/votes/votesTab.js
<h1>{t('Voted delegates')}</h1>
<div className={`${styles.filterHolder}`}>
<InputV2
disabled={votes && votes.length === 0}

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

I think this can be replace for {votes && !votes.length}

</div>
) : null
}
{filteredVotes.length > 0

This comment has been minimized.

Copy link
@osvaldovega

osvaldovega Mar 15, 2019

Contributor

I think in here too.
you can just use filteredVotes.length.
By the way, maybe you can try to use the same way in all the files when open/close brackets for dont affect the indentation.
in line 60 you open brackets and then use the next line to code but in here you open brackets and the code next to the brackets, so it is not a real problem is just that is easier to read when you do the same and not change it very often and the indentation is better to follow. Same in lines 97 and 103
(really minor details nothing related to the code) 👍🏻

@osvaldovega
Copy link
Contributor

left a comment

awesome 👍🏻

@osvaldovega osvaldovega added the ready label Mar 15, 2019

@osvaldovega
Copy link
Contributor

left a comment

looks good

@massao massao merged commit cd4aae3 into 1.14.0 Mar 15, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
security/snyk - package.json (LiskHQ) No manifest changes detected

@slaweet slaweet deleted the 1779-implement-votes-tab-on-account-page branch May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.