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

Add major selection to profile #501

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

Conversation

mjaydenkim
Copy link
Contributor

Summary

This PR works on a major update... haha i'm so funny...... anyway it allows users to set their major as a part of their account rather than each review individually (although reviews stay attached to the major selected at the time of publication).

New users are prompted to select their major on the review page with the same UI as before, but their selection will then attach to their account. Users shouldn't be shown the major selection dropdown otherwise as long as they continue to have a selected major and are logged in. Existing users can update their major on the profile page, where there's a dropdown menu to do so.

PR Type

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Mobile + Desktop Screenshots & Recordings

https://github.com/user-attachments/assets/88ed0c9b-105f-4a37-a8fc-bf0b5bad2a08
https://github.com/user-attachments/assets/8c0ca687-3eb0-4a21-aa40-6be17db0dfa1
majorupdate_ss1

QA - Test Plan

Tested in a variety of different account scenarios, including with accounts that have no major, accounts with many majors, etc. Tested new UIs on mobile, but there isn't a ton that's different. Couldn't test with a new account (b/c I don't have other NetIDs) but I simulated that performance essentially. Also tested to ensure that majors are still correctly tied to reviews.

Breaking Changes & Notes

  • All logged-out users are shown the major dropdown, and selecting a new major for an existing user notifies that user (via toast) and changes the major to the more recent selection.

Added to documentation?

  • 📜 README.md
  • 📓 notion
  • 🍕 ...
  • 📕 ...
  • 🙅 no documentation needed

What GIF represents this PR?

gif

Need to style, test. Also if major is being overwritten b/c a user was logged out, add a confirmation modal
Also got rid of a console log lol
@mjaydenkim mjaydenkim requested a review from a team as a code owner January 17, 2025 08:29
@dti-github-bot
Copy link
Member

dti-github-bot commented Jan 17, 2025

[diff-counting] Significant lines: 293.

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.

2 participants