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

Snapshot proposal enhancement #1291

Merged
merged 3 commits into from
Nov 16, 2023
Merged

Conversation

mudrila
Copy link
Contributor

@mudrila mudrila commented Oct 31, 2023

Description

This PR enhances Snapshot weighted voting support, fixing bunch of bugs around displaying such a proposal and it's results.
It also adds informational message for when vote already casted on Snapshot proposal that it's recorded but user can still re-cast the vote

Notes

Currently WIP on snapshot weighted voting casting vote

Issue / Notion doc (if applicable)

Close #1287
Close #1286
Close #1290

Testing

  1. Create snapshot weighted non-shielded voting proposal through snapshot.org
  2. Cast the vote through Fractal
  3. Make sure vote immediately appeared and tally bars are reflecting changes properly
  4. Make sure toast appeared saying that user can re-cast the vote
  5. Make sure informational message under voting choice saying that user can re-cast the vote appeared
  6. Make sure any other old weighted voting proposals are displayed properly

Screenshots (if applicable)

@mudrila mudrila added bug Something isn't working enhancement New feature or request P1 labels Oct 31, 2023
@mudrila mudrila requested a review from adamgall October 31, 2023 12:03
@mudrila mudrila self-assigned this Oct 31, 2023
@mudrila mudrila temporarily deployed to dev October 31, 2023 12:03 — with GitHub Actions Inactive
@netlify
Copy link

netlify bot commented Oct 31, 2023

Deploy Preview for fractal-framework-interface-dev ready!

Name Link
🔨 Latest commit 85b9427
🔍 Latest deploy log https://app.netlify.com/sites/fractal-framework-interface-dev/deploys/65413f8833782f0008756f33
😎 Deploy Preview https://deploy-preview-1291--fractal-framework-interface-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -43,9 +43,15 @@ export default function SnapshotProposalVotes({ proposal }: ISnapshotProposalVot
colSpan={4}
rowGap={4}
>
{choices.map(choice => {
{choices.map((choice, i) => {
const votesBreakdownChoice =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The format of votesBreakdown might be different based on snapshot proposal type, which now I think is rather bad thing. So all this crazy conditioning coming from active dev work and hot fixing arising edge cases
I'll do a follow-up PR that will cleanup extendedSnapshotProposal drilling and will move that to context, but also will cleanup generating votesBreakdown and then we should have more straightforward rendering.

justifyContent="space-between"
>
<IconButton
aria-label={`Reduce vote weight for ${label}`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We currently are not translating aria-labels


const client = new ApolloClient({
uri: 'https://hub.snapshot.org/graphql',
cache: new InMemoryCache(),
defaultOptions,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essentially, this disables the caching, cause otherwise snapshot proposal details re-fetching was returning cached results, and we wanna avoid that in order to display immediate UX feedback to user once he casted the vote on Snapshot proposal

@@ -28,6 +28,9 @@ const useCastVote = ({
setPending?: React.Dispatch<React.SetStateAction<boolean>>;
extendedSnapshotProposal?: ExtendedSnapshotProposal;
}) => {
const [selectedChoice, setSelectedChoice] = useState<number>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we'd decide to use same "selected vote option" pattern for on-chain proposals - we could easily use this same state field

setSelectedChoice(undefined);
if (onSuccess) {
// Need to refetch votes after timeout so that Snapshot API has enough time to record the vote
setTimeout(() => onSuccess(), 3000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not always, but I was facing the situation where refetching was triggered before Snapshot indexer was picking up the vote

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

LGTM

@mudrila mudrila merged commit 7bfd64e into develop Nov 16, 2023
4 checks passed
@mudrila mudrila deleted the feature/snapshot-proposal-enhancement branch November 16, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
2 participants