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 Past Enrollment Popup #820

Merged
merged 14 commits into from
Jan 25, 2024
Merged

Add Past Enrollment Popup #820

merged 14 commits into from
Jan 25, 2024

Conversation

Douglas-Hong
Copy link
Contributor

@Douglas-Hong Douglas-Hong commented Nov 30, 2023

Summary

  • Now that the enrollment history endpoint is implemented, we can bring back and improve the Past Enrollment popup.
  • Currently, the popup just displays the enrollment history of the most recent lecture section offered.

Demo:

enrollmentDemo.mp4

TODO

  • Make a request to PeterPortal to get the enrollment history, and parse the response
  • Create the popup and display an enrollment history graph
  • Cache responses
  • Add ZotTracker link

Test Plan

  • Verify this works for classes in the most recent quarter (e.g. CS 132).
  • Verify this works for classes that haven't been offered very recently (e.g. CS 122D from Spring 2023).
  • Verify this works for classes that have never been offered before (force queryEnrollmentHistory to return null).
  • Make sure it looks good on dark and light mode, as well as mobile.

Issues

Closes #341
Closes #182

Future Followup

@KevinWu098
Copy link
Member

Screenshot 2023-11-29 at 5 18 35 PM

That's seriously hot 💯

Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

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

Great feature, looks great. Only have a few readability suggestions.

Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

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

Looks great 🚀

Most of the improvements are regarding JS syntax. The comments regarding lower level implementation details can be applied at your discretion.

apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/enrollmentHistory.ts Outdated Show resolved Hide resolved
@EricPedley EricPedley removed their request for review January 24, 2024 04:36
@Douglas-Hong Douglas-Hong requested a review from ap0nia January 25, 2024 07:17
Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

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

Good work 🚀

All the comments I left were personal opinions. Feel free to peruse them before merging!

@Douglas-Hong Douglas-Hong dismissed MinhxNguyen7’s stale review January 25, 2024 22:57

Addressed changes, and approved by Brian

@ap0nia ap0nia merged commit 210298d into main Jan 25, 2024
6 checks passed
@ap0nia ap0nia deleted the enrollment-graph branch January 25, 2024 23:16
MinhxNguyen7 added a commit that referenced this pull request Jan 26, 2024
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.

Past Enrollment Modal Once PeterPortal API implements past enrollment data, uncomment the graph modal
4 participants