Added optional onDeleteVote prop to type definition, integrated delet…

[sandy4242]

Added optional onDeleteVote prop to type definition
Modified VotingPopup layout & interaction
Integrated DELETE_VOTE mutation in Post component

issue link - #383

[vercel]

@sandy4242 is attempting to deploy a commit to the Louis Girifalco's projects Team on Vercel.

A member of the Team first needs to authorize it.

[motirebuma]

Hey @sandy4242, great idea! Vote retraction plus switching is a definite UX win. One minor issue to fix before merging.

What's good

1. Backwards-compatible opt-in - onDeleteVote is optional on VotingPopupProps. If not given, we fall back to the previous behavior of the disabled button; if given, we allow either vote retraction or switching based on the existing vote. Clean!

2. Vote switching in Post.tsx - handleVoting checks if there's an existing vote of the same type (delete it, i. e. retract) or different type (delete it + add a new vote). The getUserVote helper appropriately extracts the existing vote for both the switch check and the delete call. Same pattern (update + refetchQueries) is used for removeVote as for the existing addVote.

3. Tooltip UX - the contextual tooltips ("Retract upvote", "Change vote to downvote", etc.) properly signal the intent of each button. The disabled fallback when onDeleteVote is not given is good UX as well.

4. Tests - updated to test vote retraction, vote switching, and the disabled fallback.

Items to address

1. Bug: onDeleteVote is called in the showUpvoteTooltip / showDownvoteTooltip branches - these only get evaluated when hasVoted is false (the hasVoted ternary is before them). However, the new code adds the onClick={() => { onDeleteVote?.() }} handlers to the tooltip-wrapped buttons in

~line 246 (upvote) and ~line 353 (downvote) - this would attempt to delete a vote that doesn't exist! Those buttons are supposed to prompt the selection of the corresponding vote type for users who haven't voted yet. The onDeleteVote?.() call would be a no-op (since getUserVote() would return null), but it's best to just remove those handlers entirely.

Nits (non-showstopper)

• Vote switching is not atomic - handleVoting does await removeVote(...) followed by await addVote(...). If the latter fails, the user will have to manually re-add their vote. The error toast covers the failure case, but it's still a UX hiccup.

• VotingPopup complexity - The 3-way ternary (no onDeleteVote, same vote type, different vote type) is repeated for the most part in both the upvote and downvote button groups - that's 6 blocks in total. Extracting a VoteButton that takes direction, isActiveVote, onDeleteVote, and onExpand would reduce the repetition by half. Not a showstopper, but something to consider for follow-up work.

Verdict: changes requested

Thank you @sandy4242
@flyblackbox @sandy4242

[sandy4242]

@motirebuma addressed those changes, please review once again and let me know, thank you

[motirebuma]

Hey @sandy4242, nice work, the bug from the previous review is fixed and this is ready to merge.

What's good

1. Backwards-compatible opt-in — onDeleteVote is optional on VotingPopupProps. When not provided, the old disabled-button behavior is preserved. When provided, users can retract or switch their vote. Clean API.

2. Vote switching in Post.tsx — handleVoting now checks for an existing vote: same type → delete (retract), different type → delete then add new vote. The getUserVote helper cleanly extracts the current user's vote for both the type check and the delete call. The removeVote mutation follows the same update() + refetchQueries pattern as the existing addVote.

3. Tooltip UX — contextual tooltips ("Retract upvote", "Change vote to downvote", etc.) tell the user exactly what clicking will do. The disabled state with "Vote changes are not allowed" when onDeleteVote is missing is a good fallback.

4. Bug fix — the showUpvoteTooltip / showDownvoteTooltip branches no longer call onDeleteVote?.(). The redundant if (!hasVoted) guard was cleaned up too. Good catch adding a dedicated test for this case.

5. Tests — solid coverage: retraction, vote switching, disabled fallback when onDeleteVote is absent, and the new tooltip-branch regression test.

Nits (non-blocking)

• Vote switching is not atomic — handleVoting does await removeVote(...) then await addVote(...). If the delete succeeds but the add fails, the user loses their vote. The current error toast covers the failure case, but it's worth noting as a known limitation.

• VotingPopup complexity — the 3-way ternary (no onDeleteVote / same vote type / different vote type) is duplicated nearly identically for both upvote and downvote buttons — 6 button blocks total. A VoteButton component that takes direction, isActiveVote, onDeleteVote, onExpand would cut this in half. Fine as a follow-up.

Thank you @sandy4242

@flyblackbox @sandy4242