-
Notifications
You must be signed in to change notification settings - Fork 1
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
[KD] Stylize Header and Navbar #82
Conversation
Visit the preview URL for this PR (updated for commit 1061235): https://tcl-69-smart-shopping-list--pr82-kd-stylize-navbar-6225o3cz.web.app (expires Sat, 13 Apr 2024 00:56:13 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 93172cc46147b7d365c2b1b8239b61e2efb07a80 |
…-shopping-list into kd-stylize-navbar
…attributes for responsiveness
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.
Very exciting to see the app starting to take shape design wise.
src/views/Layout.css
Outdated
|
||
.Layout-header button { | ||
background: transparent; | ||
color: var(--color-text); |
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.
Looking at the deployed it appears that this overrides the button styles in index making the text in the signin/signout button black. Suggestion is to remove this line
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.
Thanks for catching that. I'll remove that line and also change the text color of 'button' in index.css to '--color-white' so it applies to all buttons in both light and dark mode.
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.
Added a comment and suggestion to delete text
Co-authored-by: Martin Fitzpatrick <[email protected]>
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.
Good work, looks really nice! One suggestion to bring app name and navbar items a little closer to the center, things feel a bit close to the edge and navbar items feel a bit far apart.
Also, sign out button looks right for me in chrome, but looks like this in fIrefox:
I'm not sure what's causing the difference between the two browsers, would have to do more digging. Maybe @mxmason has thoughts?
update to comment above^: after setting the firefox window width to the narrowest it can be, the signout button looks fine, so I guess it's how the button is scaling up at full view? I also see now how setting the location of the app name and navbar items closer to the center might be tricky to have it both look good at the narrowest screen size and full view without getting into media queries, so probably doesn't make sense for now unless you wanted to play around with that if you have time. |
Description
This code stylizes the header, navbar, and sign-in/sign-out button using the Aisle Be There brand colors and font.
Related Issue
Closes #80
Sub-Issue #14
Acceptance Criteria
Type of Changes
Use one or more labels to help your team understand the nature of the change(s) you’re proposing. E.g.,
bug fix
orenhancement
are common ones.Updates
Before
After
Signed out
Signed in
Testing Steps / QA Criteria