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.

Sign up for GitHub

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

Jump to bottom

#561 - Overall Frontend Refactor (excludes Exercise) #586

Merged
merged 7 commits into from
Apr 17, 2024
Merged

#561 - Overall Frontend Refactor (excludes Exercise) #586

merged 7 commits into from
Apr 17, 2024

Conversation

dm9965
Copy link
Contributor

@dm9965 dm9965 commented Apr 4, 2024

  • No discrepancies across browsers. Eg. chrome vs safari
  • Pages pass W3C Validation for HTML (https://validator.w3.org)
  • CSS (https://jigsaw.w3.org/css-validator/)
  • Google sign-in is operational
  • Accessibility functions
  • Pages can scale without distorting page
  • No dead links
  • Navbar is consistent across the site
  • Pages are screen-reader accessible
  • Contrast meets standards for accessibility
  • Pages are keyboard accessible
  • Passes WAVE Evaluation Tool chrome extension

@dm9965 dm9965 linked an issue Apr 4, 2024 that may be closed by this pull request
Story: Refactor Overall Styling (Frontend) #561
Closed
22 tasks
heathermoses
heathermoses approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@heathermoses heathermoses left a comment

Choose a reason for hiding this comment

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

Small nitpicks, overall good!

client/public/index.html
Comment on lines +11 to +20
<link rel="preconnect" href="https://fonts.googleapis.com" />
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin />
<link
href="https://fonts.googleapis.com/css2?family=Montserrat:ital,wght@0,100..900;1,100..900&display=swap"
rel="stylesheet"
/>
<link
href="https://fonts.googleapis.com/css2?family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap"
rel="stylesheet"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: google fonts get imported in the main.css file. We can go back and change this later

client/src/assets/images/logos/FinalALLLogo.png
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: looks like there's two logo files, do we need both?

@Mster115
Copy link
Contributor

nit:
image
image
whole header spanning entire page while the rest of the body is stuck together doesn't flow right to me feels stretched

@Mster115
Copy link
Contributor

favicon needs to be updated with new logo. ie. one without text
nit: the header color feels out of place, idk what color would make it better. The header color seems like this is the only spot where the color is used. the footer on the homepage is also a different color so it feels off. maybe fix the footer image on main page? maybe the nav buttons on the bottom could be the same color as the nav header? again this is a nit pick
image
image

Overall it looks amazing!

@heathermoses heathermoses merged commit 43debc7 into dev Apr 17, 2024
3 checks passed
@heathermoses heathermoses deleted the 561-story-refactor-quiz-component-frontend branch April 17, 2024 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heathermoses heathermoses heathermoses approved these changes

@AndreasLc1103 AndreasLc1103 Awaiting requested review from AndreasLc1103

@Mster115 Mster115 Awaiting requested review from Mster115

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Story: Refactor Overall Styling (Frontend)
3 participants
@dm9965 @Mster115 @heathermoses