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

bug/Issue-91: Refactor headers styles to prevent logo from shrinking #113

Merged
merged 6 commits into from
Oct 18, 2024

Conversation

DevLab2425
Copy link
Contributor

@DevLab2425 DevLab2425 commented Oct 14, 2024

Related Issue

Fix #91

Summary of Changes

  • Update header styles to allow the logo to remain at a reasonable, legible size.
Screen.Recording.2024-10-14.at.19.56.17.mov

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for super-tapioca-5987ce ready!

Name Link
🔨 Latest commit a0787c6
🔍 Latest deploy log https://app.netlify.com/sites/super-tapioca-5987ce/deploys/671290ba347e0b0008e7b335
😎 Deploy Preview https://deploy-preview-113--super-tapioca-5987ce.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@DevLab2425 DevLab2425 marked this pull request as ready for review October 15, 2024 00:04
@DevLab2425 DevLab2425 added bug Something isn't working hacktoberfest-accepted Used to participate in Hacktoberfest labels Oct 15, 2024
Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

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

Nice, I think we can make one small little tweak but otherwise this looks good!

@@ -137,8 +136,9 @@
}

@media screen and (min-width: 768px) {
.logoLink svg.greenwood-logo-full {
width: 60%;
.navBar {
Copy link
Member

Choose a reason for hiding this comment

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

I might consider sizing it down a "little" bit between the 768px - 1024px range, as it does seem to dominate a bit of the header in that gap.
Screenshot 2024-10-15 at 4 12 18 PM

Maybe we can clamp it or put a max-width or something on it?

.logoLink svg.greenwood-logo-full {
width: 60%;
.navBar {
display: flex;
Copy link
Member

Choose a reason for hiding this comment

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

I think we might want keep it as display: contents at this break point as well, as the vertical alignment of the navigation looks slightly better aligned vertically with the logo and mobile menu icon (IMO 😅 )

display: flex (current)

Screenshot 2024-10-16 at 9 38 26 PM

display: contents

Screenshot 2024-10-16 at 9 39 30 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, now the nav links stay in the center as we get past tablet breakpoints, as opposed to staying to the right, against the side nav
Screenshot 2024-10-17 at 11 30 14 AM

Apologies for not catching that, I am admittedly not super well-versed with flex box. 😬

@DevLab2425 DevLab2425 force-pushed the issue-91-greenwood-logo-styles branch from e7c3191 to 602bdba Compare October 17, 2024 13:53
@DevLab2425 DevLab2425 force-pushed the issue-91-greenwood-logo-styles branch from b4683bd to 7b0bb09 Compare October 18, 2024 03:35
justify-content: space-between;
padding: 0 var(--size-4) var(--size-2);
margin: 0;
justify-content: center;
Copy link
Member

@thescientist13 thescientist13 Oct 18, 2024

Choose a reason for hiding this comment

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

The (bottom) padding / margin removal has made things a bit more cramped at the bottom of the header especially on mobile
Screenshot 2024-10-18 at 12 44 08 PM

Made one small tweak to retain some of that and now I think we're good to go!
Screenshot 2024-10-18 at 12 44 13 PM

Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

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

Thank you sir! 🫡

@thescientist13 thescientist13 changed the title Issue-91: Refactor headers styles to prevent logo from shrinking bug/Issue-91: Refactor headers styles to prevent logo from shrinking Oct 18, 2024
@thescientist13 thescientist13 merged commit 9181075 into main Oct 18, 2024
5 checks passed
@thescientist13 thescientist13 deleted the issue-91-greenwood-logo-styles branch October 18, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest-accepted Used to participate in Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

greenwood header logo is inconsistently sized on medium / tablet breakpoints
2 participants