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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delegate Vote Refactor #491

Merged
merged 16 commits into from Jan 8, 2018

Conversation

Projects
None yet
5 participants
@perryhoffman
Contributor

perryhoffman commented Dec 28, 2017

I have been meaning to make this PR for a while now! It's been tricky with the busy holiday season. There is still a lot of room for improvement on this, but with the ARK community getting larger by the day I feel like the changes are a necessary step in the right direction. The issue is regarding the current state of delegate voting, and the confusion surrounding the user experience. Various issues I have seen include this Reddit Post, the UI changes referred to in #346, and a few others.

The changes to the UI are as follows:

screen shot 2017-12-28 at 2 52 07 am

screen shot 2017-12-28 at 2 36 50 am

screen shot 2017-12-28 at 2 37 13 am

screen shot 2017-12-28 at 2 36 32 am

A tooltip has been added to the disabled Vote button if there is an active delegate explaining only 1 delegate can be active at a time. There was also a bug with the theme of the vote dialog template that was fixed. I also refactored the logic surrounding the vote/unvote methods out of the main account.controller in the process.

Some help testing this and feedback on this is welcomed! Shoutout to @heyhaigh on slack for the consulting 馃憤 .

@j-a-m-l j-a-m-l self-requested a review Dec 29, 2017

@j-a-m-l

j-a-m-l requested changes Dec 31, 2017 edited

Looks very promising. Removing the confusion that generates having both "Add delegate" and "Vote" buttons is a very good idea.

Since this is very related to the purpose of this PR, would you mind to check #479. Adding a link would suffice probably.

@perryhoffman

This comment has been minimized.

Show comment
Hide comment
@perryhoffman

perryhoffman Jan 1, 2018

Contributor

@j-a-m-l I added the link as an immediate solution, solving for #479. @heyhaigh has been helping me on slack design a more elegant solution for this user experience. Hopefully I can get more PR's ready in the coming weeks to further improve the delegate vote flow.

Contributor

perryhoffman commented Jan 1, 2018

@j-a-m-l I added the link as an immediate solution, solving for #479. @heyhaigh has been helping me on slack design a more elegant solution for this user experience. Hopefully I can get more PR's ready in the coming weeks to further improve the delegate vote flow.

@j-a-m-l

This comment has been minimized.

Show comment
Hide comment
@j-a-m-l

j-a-m-l Jan 5, 2018

Member

@perryhoffman do you need some help? (I don't know which is your Slack user)

Member

j-a-m-l commented Jan 5, 2018

@perryhoffman do you need some help? (I don't know which is your Slack user)

@heyhaigh

This comment has been minimized.

Show comment
Hide comment
@heyhaigh

heyhaigh Jan 5, 2018

He's @pjh on ARK Slack, @j-a-m-l

heyhaigh commented Jan 5, 2018

He's @pjh on ARK Slack, @j-a-m-l

@j-a-m-l

j-a-m-l approved these changes Jan 6, 2018

@j-a-m-l

This comment has been minimized.

Show comment
Hide comment
@j-a-m-l

j-a-m-l Jan 6, 2018

Member

Thanks @heyhaigh, I contacted @perryhoffman.

@perryhoffman this PR seems good enough, so you can resolve the conflicts + refactor if you want and I'll merge it.

Member

j-a-m-l commented Jan 6, 2018

Thanks @heyhaigh, I contacted @perryhoffman.

@perryhoffman this PR seems good enough, so you can resolve the conflicts + refactor if you want and I'll merge it.

@luciorubeens

I tried to vote/unvote but nothing happens.

@Nasicus

This comment has been minimized.

Show comment
Hide comment
@Nasicus

Nasicus Jan 8, 2018

Contributor

... and now that he has fixed conflicts for the 10th time, and is ready, it's time to merge my ESLint task back first 馃槇 馃槇 馃槇 馃槇 馃槇 馃槇 馃槇

image

Contributor

Nasicus commented Jan 8, 2018

... and now that he has fixed conflicts for the 10th time, and is ready, it's time to merge my ESLint task back first 馃槇 馃槇 馃槇 馃槇 馃槇 馃槇 馃槇

image

@luciorubeens luciorubeens merged commit fa93f32 into ArkEcosystem:master Jan 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Jan 8, 2018

Member

Great! 馃憤

Member

luciorubeens commented Jan 8, 2018

Great! 馃憤

@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Jan 8, 2018

Member

@Nasicus Let's discuss this :)

Member

luciorubeens commented Jan 8, 2018

@Nasicus Let's discuss this :)

@heyhaigh

This comment has been minimized.

Show comment
Hide comment
@heyhaigh

heyhaigh Jan 8, 2018

Can't wait to use this, gents. Well done.

heyhaigh commented Jan 8, 2018

Can't wait to use this, gents. Well done.

@perryhoffman

This comment has been minimized.

Show comment
Hide comment
@perryhoffman

perryhoffman Jan 9, 2018

Contributor

@Nasicus haha, no please, no more resolves! Thanks @luciorubeens for the merge 馃憣.

Contributor

perryhoffman commented Jan 9, 2018

@Nasicus haha, no please, no more resolves! Thanks @luciorubeens for the merge 馃憣.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment