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

fixing css issues #251

Merged
merged 5 commits into from
Oct 26, 2018
Merged

fixing css issues #251

merged 5 commits into from
Oct 26, 2018

Conversation

pqv199x
Copy link
Contributor

@pqv199x pqv199x commented Oct 25, 2018

image
image
image
Candidate transactions:
image
Voter transactions:
image

@coveralls
Copy link

coveralls commented Oct 25, 2018

Pull Request Test Coverage Report for Build 336

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 79.47%

Totals Coverage Status
Change from base Build 327: 0.0%
Covered Lines: 92
Relevant Lines: 103

💛 - Coveralls

Copy link
Contributor

@etienne-napoleone etienne-napoleone left a comment

Choose a reason for hiding this comment

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

Thanks!
Can you also check that:


Is it possible to remove ID column in candidate Signs? So a few seconds ago can be on one line?
image
The ID isn't meaningful, it's just 1 to 10 on each page.


Is it also possible to have the ID/Age Sorting on the voters transactions?


Lastly, do you think it's possible to fix the spacing here?
image
I think it should be the same as between the network status blocks or the same as the space between each rows

@pqv199x
Copy link
Contributor Author

pqv199x commented Oct 25, 2018

Yeah, np, can you check it for me
image

image
I can remove the ID column, but should I ? I updated ID column, so it can mark the number in order

@etienne-napoleone
Copy link
Contributor

Not sure, I think the ID collumn is useless here because it doesn't represent anything (click on page 2, it will still be ID 1 to 10). But we can keep it I guess.

But I see you made the opposite of what I thought, you made the spacing between row the same as the spacing between "masternode" and "vote" instead of the opposite, can you change that? I think it takes too much space.

@thanhson1085
Copy link
Contributor

Yep, vote for removing id column

@etienne-napoleone
Copy link
Contributor

@pqv199x did you fixed the spacing on the status and vote button?

@khaihkd
Copy link
Contributor

khaihkd commented Oct 26, 2018

I think we remove ID column in homepage, candidate detail, and replace withNumber column, Number will grow up from 1 to n. It is just for count

@etienne-napoleone
Copy link
Contributor

etienne-napoleone commented Oct 26, 2018

@khaihkd I agree, the ID column is actually always useless and give no useful information. I think it could be good to remove it everywhere.
I don't think a Number column would be usefull tho. it will just 1-10 all the time :/

@etienne-napoleone
Copy link
Contributor

Also the ID on the Candidate list could be named "Rank" instead and be tied to their rank

@pqv199x
Copy link
Contributor Author

pqv199x commented Oct 26, 2018

@etienne-napoleone I returned the spacings as they were, it's quite complex for me to fix them now

@thanhson1085
Copy link
Contributor

merged it, for fixing the space, do in another PR

@thanhson1085 thanhson1085 merged commit bd7f0b7 into BuildOnViction:master Oct 26, 2018
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

5 participants