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

PCP - map feature #684

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

PCP - map feature #684

wants to merge 21 commits into from

Conversation

jamesdoh0109
Copy link

PR for the new map feature (PCP)

  • Hyperlink for building names -> modal popup w/ map on click
  • New "map" tab along with "cart" and "alert", where you can see course locations for a particular day on your schedule (toggle b/w different days)
  • Color coded pins allow you to match a course on the map with the corresponding course in schedule
  • Apply certain delta value amongst course pins on the map that are in the same building so that users can see clear separation
Screenshot 2024-11-10 at 1 08 45 PM

Copy link
Contributor

@Clue88 Clue88 left a comment

Choose a reason for hiding this comment

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

This looks awesome, James! Thanks for all of your work on this feature. Besides the merge conflict with the Pipfile, this looks good to me on a high level—I would make sure to get a review from Luke or Ryder before merging though just to confirm that all of the frontend stuff is all set.

Copy link
Contributor

@shiva-menta shiva-menta left a comment

Choose a reason for hiding this comment

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

Great work James! Excited to see this hit production. I just reviewed the backend and made some minor changes, as well as updated the production database to include course locations, so this feature will have data to work off of for prod.

Backend looks good on my side, I'll leave frontend up to Luke and Ryder.

I tried running the frontend on my side and faced some issues in the image below – not sure if this is just an issue on my side, so I'd appreciate if you could take a look at it before merging.

Screenshot 2024-11-11 at 1 57 12 PM

Here's a way to replicate the issue:

  • Checkout to master, run the frontend / backend, add 1-2 courses to your cart and have them show up on the schedule
  • Checkout to pcp_doh_newbie_location, run the frontend / backend – the issue should show up.

Seems like this is likely an issue with cached data not having some attribute set up right – in the above image color is null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants