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

Optimize upvote and downvote colors #364

Closed
wants to merge 3 commits into from

Conversation

advayanand
Copy link
Collaborator

Description

  • An API request is made for only a single review's vote every time it is upvoted or downvoted, instead of for all reviews.
  • Changes to vote color and score instantly reflect when a vote is cast, instead of after the API response, which used to cause a delay. If the vote fails to go through, the color and score are reverted to their original state.

Screenshots

Screen.Recording.2023-10-13.at.5.17.28.PM.mov

Steps to verify/test this change:

  • Verify changes work as expected on staging instance

Final Checks:

  • Verify successful deployment
  • Delete branch

(optional)

  • Write tests
  • Write documentation

Issues

Closes #

@advayanand advayanand linked an issue Oct 14, 2023 that may be closed by this pull request
@github-actions
Copy link

Deployed staging instance to https://staging-364.peterportal.org

if (!cookies.hasOwnProperty('user')) {
alert('You must be logged in to vote.');
return;
}
const votes = {
id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,
// id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,

Copy link
Member

@js0mmer js0mmer left a comment

Choose a reason for hiding this comment

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

Looks good. See comments regarding code. Also, I merged in my review sorting PR so it looks like there are merge conflicts now. Lmk if you have any questions about them or need help with them.

}
let colors = await getColors(req);
console.log(colors);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log(colors);

@@ -61,14 +66,30 @@ const SubReview: FC<SubReviewProps> = ({ review, course, professor, colors, colo
return;
}
const votes = {
id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,
// id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// id: ((e.target as HTMLElement).parentNode! as Element).getAttribute('id')!,

site/src/component/Review/Review.tsx Show resolved Hide resolved
Copy link
Contributor

@SenghoungLim SenghoungLim left a comment

Choose a reason for hiding this comment

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

Tested on the instance, it runs better now. Besides Jacob's comments, I think the code looks good. Good job.

@js0mmer js0mmer self-requested a review January 18, 2024 20:08
Copy link
Member

@js0mmer js0mmer left a comment

Choose a reason for hiding this comment

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

Address the previous comments and you should be good to go!

@advayanand advayanand closed this Feb 14, 2024
@js0mmer js0mmer deleted the advay/vote-colors-optimization branch February 20, 2024 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize upvote/downvote colors
3 participants