-
Notifications
You must be signed in to change notification settings - Fork 7
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
Snapshot proposal breakdown and votes fixes #2350
Conversation
…viding info about voting weight per strategy for the vote
✅ Deploy Preview for decent-interface-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks @mudrila, the multiple voting options in the results looks good.
I'm afraid I don't know enough about the multiple voting strategies to be able to provide and concrete review or direction here. I'm guessing multiple strategies = multiple NFTs? Is voting happening in our UI or in Snapshot? How can I test this all out myself?
The UI showing what I am guessing are multiple NFTs being used to vote is a bit confusing to me — idk how the different NFTs affect the vote here. But beyond that, I can't give this any further formal review.
Not only NFTs - these "symbols" are representing different voting strategies (better to say different voting power calculation strategies) within Snapshot. You can use multiple snapshot voting strategies - like let's say you wanna enable voting power for multiple ERC-20s, or multiple ERC-721, or even both ERC-20 and ERC-721 and ERC-1155 and special voting power to council members - but also provide different voting weight for holders of different tokens / members of different groups. What you see on the screen is proposal that represents setup of So now, The row with The goal of this PR, mostly - to slightly enhance representation comparing to the old one. Previously it was just a string with first strategy - and that is simply wrong, since more strategies could be used for the specific proposal. Also this PR outlines which voting strategies are counted towards certain user's voting weight - aka "why da heck this user has such high voting power". Hope this Snapshot Alcoholic explanation makes sense 🤣 |
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.
Thanks for the breakdown @mudrila
I'm approving but I honestly still don't fully grasp what needs to be communicated to the user effectively here. Everything you described makes sense, and I see all the pieces on the UI here but I need to understand the context more to make any further suggestions.
IDK where this voting setup work lives in our priorities after MCon, but this is something I'd like to revisit with a clearer scope of work down the road :-)
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.
Ha funny that I just fixed the NaN bug on the typechain branch. Thought is was something I did. We can just "accept all" from the PR during merge conflict resolution
NaN
in the breakdown when there're no votes on snapshot proposalNote problem with the breakdown:
Note how we weren't showing all the voting strategies that are applicable for the proposal and voting weight per strategy on the specific vote entry:
Updated implementation: