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

feat: Class Search Shortcut #1092

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

feat: Class Search Shortcut #1092

wants to merge 13 commits into from

Conversation

alexespejo
Copy link
Contributor

@alexespejo alexespejo commented Dec 30, 2024

Summary

Users can now quickly search for courses they've selected by clicking a shortcut in in their calendar

A new button is added to the course popup when you click on the calendar event which will auto search the course offerings. Additionally, cmd/ctrl + click on a calendar event will perform the search.

This is meant to enhance the user experience by reducing the redundancy of having re-search a course if a user wants to select or preview different time slots

image
Screen.Recording.2024-12-29.at.8.26.15.PM.mov

Test Plan

  1. Add different lec/dis/sem to the calendar, click the calendar event and click the title in the popup to search for the course will bring up the course and all associated disc/lecs
  2. Do the same for 1. but use cmd/ctrl + click to search the course
  3. If you're on the Added or Map pages and you try either 1. or 2. nothing should happen
  4. Using the shortcut and searching shouldn't affect either actions (this was a problem in development)

Issues

  • This feature is locked for mobile to avoid clunky behavior
  • The search is only preformed when the user is on the Search tab. If the user is on the 'Added' or 'Map' the search won't display (this is subject to change based on how the feature feels)

Closes #1046

@alexespejo alexespejo marked this pull request as ready for review December 30, 2024 04:12
Copy link
Member

@KevinWu098 KevinWu098 left a comment

Choose a reason for hiding this comment

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

Solid PR, I think users will be really happy with this improvement to the user flow around the app.

That said, I have a few questions about the approach we're taking in this PR. More specifically, I don't feel that we need to use a new store in order to implement the logic for this feature.

Short of refactoring RightPaneStore to a Zustand store, I think this PR should reuse logic from CourseInfoSearchButton and utilize RightPaneStore and CoursePaneStore over creating another store with duplicate states

apps/antalmanac/src/stores/QuickSearchStore.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/helpers.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/helpers.ts Outdated Show resolved Hide resolved
apps/antalmanac/src/lib/helpers.ts Outdated Show resolved Hide resolved
@alexespejo
Copy link
Contributor Author

Solid PR, I think users will be really happy with this improvement to the user flow around the app.

That said, I have a few questions about the approach we're taking in this PR. More specifically, I don't feel that we need to use a new store in order to implement the logic for this feature.

Short of refactoring RightPaneStore to a Zustand store, I think this PR should reuse logic from CourseInfoSearchButton and utilize RightPaneStore and CoursePaneStore over creating another store with duplicate states

Apologies for the slow development speed

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.

Adding Shortcuts for accessing courses
2 participants