-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: add reason when voting #511
Conversation
Issue about voting power fixed. Tested and confirmed the reason submission is working on starknet and offchain, but still not able to test on evm, as eth-sepolia not working, and other l2 networks have insufficient funds. |
Finally tested on our own new sepolia graph, it's working well. |
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.
Removing close trigger for #386 because this PR is not covering entire issue (progress, share etc.)
availableChoices: string[], | ||
choice: number | number[] | Record<string, number> | ||
) { | ||
export function getChoiceText(availableChoices: string[], choice: Choice) { |
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.
Not sure why those changes were introduced in this PR?
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.
We use getChoiceText()
to preview the user's choice in the vote modal.
The previous function does not return anything when the choice is invalid or blank (in the case of weighted choice)
For some reason, when I create a proposal with this PR, I stay on the editor instead of being redirected to the proposals page. I also can change the voting type while it shouldn't be the case on the space I've enforced "basic" voting type, this space: http://localhost:8080/#/s:fabien.eth In the main branch I don't see these issues. Otherwise the modal looks good. |
Co-authored-by: Wiktor Tkaczyński <[email protected]>
…labs/sx-monorepo into feat-vote-reason-modal
Co-authored-by: Wiktor Tkaczyński <[email protected]>
…labs/sx-monorepo into feat-vote-reason-modal
There's not need to check for authInitiated, as we don't support login and logout while in this page
Just noticed that this PR is currently only checking that voting power is > 0 to vote, and does not take into consideration when a space has set a custom minimum voting power score. Will work on that |
After some progress, may be better to have it in another PR, as this PR is already complex enough. Seems like it's not a matter of adding 2 more lines |
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.
tACK, other than one small comment, needs rebase as well.
Co-authored-by: Wiktor Tkaczyński <[email protected]>
Summary
Re-submission of the vote reason modal PR (#427), which was reverted
Towards: #386
This PR moves the vote UI inside a vote modal, similar to v1, and allow attaching a reason to a vote.
Features
reason
field has been added in the vote modal, where the voter can attach a reason to his vote.getVotingPower
logic into a store and composable, similar to how we handleSpace
, enabling caching and avoid code duplicateHow to test
CONFIRM
button should be disabled if you don't have sufficient voting power to votemetadataUri
(onchain) orreason
(offchain) field if setRETRY
button to refetch the voting power, in case of fetching errorTODO
Future