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

Improve review UI (a sequel to #545: Fix rating review overflow) #552

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

charlieweinberger
Copy link

@charlieweinberger charlieweinberger commented Jan 14, 2025

Improved several aspects of PeterPortal reviews.

Description

This pull request is a continuation of pull request #545: Fix rating review overflow. As you’ll see, I made a lot more changes than just fixing the rating overflow, so I figured it’d be better to make a new branch. I do apologize in advance for the mess that I’ve caused, creating multiple pull requests and whatnot. Hopefully, it all works out in the end, with the best possible code being the output. In the future, I'll try to stick more closely with the corresponding pull request I'm fixing, and just create more issues whenever I find things that I think could be improved. I already have more ideas of how to improve reviews, but I didn't want to implement anything TOO drastic.

After pushing code to pull request #545 a few hours ago, I continued to work on the UI/UX for reviews, because while working I noticed many things that could be improved, and I was in the mood to fix them. I will admit, I got carried away with it, and many of the changes I made are not related to the original issue. However, I do believe that these changes are an improvement to the current UI/UX, and I hope you agree with me. Let me know your thoughts.

Note that this version that I’m committing is not ready to be merged. As you’ll see when you check it out, it has many red boxes, although they may be commented out. I use these to help me identify both the boundaries of elements, and what needs fixing. The latter will apply to you - anything that has a box around it still either needs to be fixed, or at least has a question corresponding to it. I’m mainly looking for feedback on my changes, which I can then implement, and THEN merge to main.

Things I fixed

  • Fixed responsiveness issues from the previous commit (pull request Fix rating review overflow #545).
  • Fixed/improved many responsiveness issues by largely replacing the use of margin w/ flexboxes.
  • Moved review date into 2nd subreview details element, above the now-named “quarter” and “grade” labels, to match the Figma design.
  • Updated the color of review tags to be peterportal-primary-color-1, to match the Figma design.
  • Turned the report and add review buttons into react-bootstrap buttons.
  • Improved the report button UI by fixing the text color when in light mode and adding rounded edges
  • Removed a bug where the horizontal spacing between the sort/filter dropdowns changed when the height of the window changed.
  • Vertically centered the voting elements, to align with the report button.
  • Updated the “show verified reviewers only” checkbox from the semantic-ui component to the react-bootstrap component, vertically centered it to align with the dropdowns, and fixed the coloring issues that followed.
  • Below 600px, the sorting menu is now 3 rows, one for each dropdown/checkbox.
  • Renamed and reordered CSS classes for clarity.

Screenshots

Before:
Screenshot 2025-01-17 at 10 18 31 AM
Screenshot 2025-01-17 at 10 18 37 AM

After:
Screenshot 2025-01-17 at 10 21 34 AM
Screenshot 2025-01-17 at 10 21 41 AM

Test Plan

  • Take a look at my changes and compare them to current PeterPortal reviews.
  • Test reviews in both light & dark mode.
  • Test reviews anywhere they can appear (courses, professors, and the 'unverified reviews' tab).
  • Test reviews for all window sizes (anywhere from 1600px to 400px), with a focus on the breakpoints of 1300px and 600px.

Issues

Closes issue #304 and pull request #545

@laggycomputer
Copy link
Member

laggycomputer commented Jan 14, 2025

Note that this version that I’m committing is not ready to be merged.

Should this be marked as a draft?

@charlieweinberger
Copy link
Author

Note that this version that I’m committing is not ready to be merged.

Should this be marked as a draft?

I didn't know that was something you could do for pull requests, but yes, that would seem to apply.

@charlieweinberger charlieweinberger marked this pull request as draft January 14, 2025 19:17
@charlieweinberger
Copy link
Author

Just converted this to a draft, thanks for the heads up. However, I'd still like others (mainly @Awesome-E) to review this pr and let me kow their feedback.

…rimary to reflect the Figma design, and made improvements to the sorting/filtering menu
@charlieweinberger charlieweinberger marked this pull request as ready for review January 19, 2025 22:31
Copy link
Member

@Awesome-E Awesome-E left a comment

Choose a reason for hiding this comment

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

image the check box should be inside the label, otherwise clicking on the text to select the checkbox no longer works

site/src/component/Review/Review.scss Outdated Show resolved Hide resolved
site/src/component/Review/Review.scss Show resolved Hide resolved
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.

In user Review page, there is an overflow the "Quality" & "Difficulty" heading
3 participants