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.

Sign up for GitHub

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

[ENG-4] Ensure Voting Button disappears after voting #2688

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

[ENG-4] Ensure Voting Button disappears after voting #2688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e810514
Enhance proposal action visibility logic to account for voting status
Da-Colon Jan 24, 2025
3da1bed
Refactor VoteContext to load snapshotProposal alongside voting weight
Da-Colon Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/components/Proposals/ProposalActions/AzoriusOrSnapshotProposalAction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,23 @@ export function AzoriusOrSnapshotProposalAction({
} = useFractal();
const { t } = useTranslation();
const { snapshotProposal } = useSnapshotProposal(proposal);
const { canVote } = useVoteContext();
const { canVote, hasVoted } = useVoteContext();

const isActiveProposal = useMemo(
() => proposal.state === FractalProposalState.ACTIVE,
[proposal.state],
);

const showActionButton =
(snapshotProposal && canVote && isActiveProposal) ||
isActiveProposal ||
proposal.state === FractalProposalState.EXECUTABLE ||
proposal.state === FractalProposalState.TIMELOCKABLE ||
proposal.state === FractalProposalState.TIMELOCKED;
const showActionButton = useMemo(() => {
const isSnapshotProposal = snapshotProposal && canVote && isActiveProposal && !hasVoted;
Copy link
Contributor

Choose a reason for hiding this comment

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

On snapshot proposal you can re-do your vote - that was intentional to keep the button, cause comparing to Azorius / Multisig - you can not re-cast your vote.

Except for ERC-721 - there's a case when your voting power changed during the vote, and then you can vote once more (even for different option!) with the remaining tokens (aka tokens that weren't used during last time you've casted your vote).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh? lol Okay gonna need to have to write this all out on what states and what we want it to look like @nicolaus-sherrill

const isAzoriusProposal = canVote && isActiveProposal && !hasVoted;
Copy link
Contributor

Choose a reason for hiding this comment

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

So yeah - with ERC-721 it's a little more tricky

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably should rely just on canVote && isActiveProposal. I'd expect that canVote returns false for ERC-20 when you already voted, but true for ERC-721 if you have remaningTokensIds.length > 0.

const isOtherProposalStates =
proposal.state === FractalProposalState.EXECUTABLE ||
proposal.state === FractalProposalState.TIMELOCKABLE ||
proposal.state === FractalProposalState.TIMELOCKED;

return isSnapshotProposal || isAzoriusProposal || isOtherProposalStates;
}, [snapshotProposal, canVote, hasVoted, isActiveProposal, proposal.state]);
Copy link
Contributor

Choose a reason for hiding this comment

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

lool I see

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, probably memoizing would solve that flickering issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll memoize anyway just a good idea in general but I didn't notice any flickering. but don't doubt it


const label = useMemo(() => {
if (snapshotProposal) {
Expand Down
3 changes: 1 addition & 2 deletions src/components/Proposals/ProposalVotes/context/VoteContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ export function VoteContextProvider({
const { governance } = useFractal();
const userAccount = useAccount();
const { safe } = useDaoInfoStore();
const { loadVotingWeight } = useSnapshotProposal(proposal as SnapshotProposal);
const { loadVotingWeight, snapshotProposal } = useSnapshotProposal(proposal as SnapshotProposal);
const { remainingTokenIds } = useUserERC721VotingTokens(null, proposal.proposalId, true);
const { snapshotProposal } = useSnapshotProposal(proposal);
const publicClient = useNetworkPublicClient();

const getHasVoted = useCallback(() => {
Expand Down