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

cmd-f 2024 Hacker App changes #565

Merged
merged 26 commits into from
Feb 10, 2024
Merged

cmd-f 2024 Hacker App changes #565

merged 26 commits into from
Feb 10, 2024

Conversation

meleongg
Copy link
Member

@meleongg meleongg commented Feb 9, 2024

Description

Other considerations

  • band-aid fix currently using nwHacks theme in ThemeProvider.js -> switch this to cmd-f when official portal reskin is ready and switch the logos to cmd-f
  • lots of tech debt/repeated code in Questionnaire.js, ReviewCards.js

Copy link

github-actions bot commented Feb 9, 2024

Visit the preview URL for this PR (updated for commit fbf60c9):

https://nwplus-ubc--pr565-cmd-f2024-hacker-app-ik2dm94d.web.app

(expires Sat, 17 Feb 2024 20:18:04 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 8c7ea898e009e43455645bc310bcbccfc0f87e48

@michelleykim
Copy link
Contributor

The questions look good to me, but just a nitpick- on mobile the checkmark for some questions doesnt show up and its hard to see what is selected and what is not. But this change can totally be made after we meege the changes for 7pm release
Screenshot_20240209_174327_Samsung Internet

@DonaldKLee
Copy link
Member

are we merging rn or right at 7?

@michelleykim
Copy link
Contributor

michelleykim commented Feb 10, 2024

are we merging rn or right at 7?

Doesn't matter. We definitely don't have to wait til 7, but we can take time

@meleongg meleongg merged commit 33a01a0 into dev Feb 10, 2024
2 checks passed
@meleongg meleongg mentioned this pull request Feb 10, 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.

5 participants