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
Fix comments bugs #2384
Merged
tiagoalvesdulce
merged 12 commits into
decred:master
from
tiagoalvesdulce:tiago_fix_commentsort
May 11, 2021
Merged
Fix comments bugs #2384
tiagoalvesdulce
merged 12 commits into
decred:master
from
tiagoalvesdulce:tiago_fix_commentsort
May 11, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ve approach with comments
SYNC_LIKE actions can be removed. I think they are pointless. |
amass01
approved these changes
May 11, 2021
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.
LGTM, nice job!
lukebp
approved these changes
May 11, 2021
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.
ACK
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2374
Closes #2375
This was a tough one to debug...
Solution description
Comment sorting (#2374)
Comment sorting was wrong because
resultvotes
wasn't being updated properly in the comments reducer. I moved it from there to theContainer
level because we don't need to store it at all since we can easily calculate it using up and downvotes.Wrong vote color (Scenario 1 on #2375)
The
useComments
hook was passingactionData.data
as the vote option to the Likes component but the right property isvote
. Changing it toactionData.vote
fixed the issue.Wrong vote count (Scenario 2 on #2375)
The
RECEIVE_LIKED_COMMENTS
action on comments reducer was using a wrong property name to store commentsLikes.Unnecessary component updates
While working on this issue I noticed some unnecessary updates when hovering the vote button. See:
This was happening because we were using a javascript approach with the useHover hook from
pi-ui
. In this specific case I could come up with a CSS-only solution to deal with hover and prevent unnecessary component updates.Wrong behavior on error (tested max number of vote changes)
I would like a review from @victorgcramos on this one because he was the last one to touch this piece of code. I don't get why we need
RESET_SYNC_LIKE_COMMENT
and why we are dispatching it on error. This was leading to some weird errors when hitting the max number of votes error.Improve code comments
The function that was calculating
upvotes
anddownvotes
based on the user previous vote had one edge case wrong and was very hard to understand/debug. I refactored it and add some comments.Comment vote UX (#2375 (comment))
Removed the not-allowed symbol during api loading and added a 500ms debounce to the voting functions so the user can't spam the button.