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

Fixed prereq box to only appear when there is text #367

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

kylerpan
Copy link
Contributor

@kylerpan kylerpan commented Oct 17, 2023

Description

This PR is for issue #356

  • Checked if there was prerequisite text for the class, if not, then hide the prerequisite text box

Screenshots

Before
Screenshot 2023-10-17 at 4 31 51 PM
After
Screenshot 2023-10-17 at 4 32 13 PM

Steps to verify/test this change:

  • Verify changes work as expected on staging instance

Final Checks:

  • Verify successful deployment
  • Delete branch

(optional)

  • Write tests
  • Write documentation

Issues

Closes #

@github-actions
Copy link

Deployed staging instance to https://staging-367.peterportal.org

@kylerpan kylerpan requested a review from js0mmer October 18, 2023 00:03
@kylerpan kylerpan self-assigned this Oct 18, 2023
@js0mmer js0mmer linked an issue Oct 18, 2023 that may be closed by this pull request
Copy link
Member

@js0mmer js0mmer left a comment

Choose a reason for hiding this comment

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

LGTM!

@js0mmer js0mmer merged commit 03783f9 into master Oct 20, 2023
2 checks passed
@js0mmer js0mmer deleted the kyle/fix-empty-prerequisite-box branch October 20, 2023 03:29
js0mmer added a commit that referenced this pull request Jan 21, 2024
* feat: initial migrate test

* feat: stable ground (no more ts errors)

* feat: everything works(?)

* fix: update footer

* feat: more cleanup

* fix: properly case GitHub

* fix: sort grade dist quarter dropdown, misc fixes

* fix: use async because it is almost 2024

* fix: address most comment changes

* fix: show class title in prereq tree nodes, correct links

* fix: less bad solution for prereq titles

* feat: fetch course/instructor data on demand

* fixed prereq box to only appear when there is text (#367)

* fix: grab review form fix from #375

Co-authored-by: Jacob Sommer <[email protected]>

* feat: use graphql for courses again, set endpoint to staging

* chore: migrate to latest api staging

* fix: make prereq tree node labels consistently spaced

* fix: reincorporate #391 changes

* feat: migrate to latest staging, hopefully fix perf issues

* Fix review form merging

* Restore original transformation fucntions

* Restore API url env vars

---------

Co-authored-by: Kyle Pan <[email protected]>
Co-authored-by: Jacob Sommer <[email protected]>
Co-authored-by: Jacob Sommer <[email protected]>
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.

Don't show empty prerequisite box
2 participants