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

Edit program page & Navbar #65

Merged
merged 29 commits into from
Feb 9, 2025
Merged

Edit program page & Navbar #65

merged 29 commits into from
Feb 9, 2025

Conversation

kaitlinnleung
Copy link
Contributor

Description: Create Edit Program page and edit Navbar

Screenshots/Media

Issues

Closes #58

@theNatePi
Copy link
Collaborator

Note: Fix yarn + npm conflict on merge

Copy link
Collaborator

@jessieh9 jessieh9 left a comment

Choose a reason for hiding this comment

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

Hi guys, just a couple changes here:

  • Do not use npm for your packages. You will need to remove package-lock.json and reinstall all your packages using yarn. Here is a code snippet of how to revert to yarn
git checkout origin/main -- package-lock.json yarn.lock 
rm package-lock.json
rm -rf node_modules
yarn install
git add package.json yarn.lock
git commit -m "Restore yarn.lock and remove package-lock.json"

After doing this, all the packages you installed using NPM will be gone. You will need to re-install those packages for the site to work.

  • For the navbar component, if I scroll down, it seems that it doesn't stretch all the way down to the page.
image
  • Also, a small fix we can try to make is font weight of the Links in the navbar to match design more (try 500 or so)
  • For the Close button, let's try to get the popup closer into the page so it doesn't trail off, matching the Figma
image
  • Small fix is to remove variables, imports, and things not being used

Other than that, great work on the modals, it was a large amount of data and the functionality is great.

@theNatePi
Copy link
Collaborator

Yarn lock still showing npm, is this an issue @jessieh9?

@theNatePi
Copy link
Collaborator

theNatePi commented Feb 9, 2025

Some outstanding issues, but overall looks good to me! Nice work

jessieh9
jessieh9 previously approved these changes Feb 9, 2025
@jessieh9 jessieh9 merged commit cabc4b9 into main Feb 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Edit Program Page
3 participants