-
Notifications
You must be signed in to change notification settings - Fork 73
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
Navigation fixes #349
Navigation fixes #349
Conversation
.top-bar-section { | ||
overflow: hidden; | ||
height: 0px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule declaration should be followed by an empty line
Hey, @johnthepink - thanks for these changes! 🎊 🎉 This definitely removes the margin issue, but it also makes a slight design change. With the existing code, the edge of the nav bar falls right below the word "montage" in the logo: and in your screenshots the edge moved above the word "montage". Could you use a media query to ensure the margin only changes in the small version of the app? Thank you! Additionally, before this is QA-ready, it would be great if you could make a few other changes:
Sorry to overwhelm with the adjustment asks. I appreciate you taking the time to try to fix this bug. Don't hesitate to let me know if you have any questions! |
No worries! Thanks for the feedback. I'd love to make the changes you suggested, I'm just not sure I completely understand what you're going for. Are you saying you want the logged out nav to be the same as it currently is, and then fix the logged out mobile spacing issue? When I made the logged out top margin the same as the logged in, it didn't leave enough room for the logo to sit in the same place in relation to the end of the nav. Sorry for the confusion 😸 |
@courte just realized you may not have seen my last comment since I didn't tag you. |
Sorry for the delay, @johnthepink! Yes - you're right on track. I'd like the logged out nav to stay the same, and then just fix the mobile spacing issue, which will probably require setting the margin change inside of a media query. 👍 |
Ok awesome @courte. I will fix this up soon. |
I'm going to close this and start over. |
Were you thinking more like this for the navigation spacing when logged out? I also found some responsive issues and fixed those as well.
Logged Out Desktop:
Logged Out Mobile:
Logged Out Mobile Expanded:
Logged In Desktop:
Logged In Mobile:
Logged In Mobile Expanded: