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

Update Navbar with options for logo widths and max width for mobile #1313

Closed
slowbot opened this issue Jun 3, 2021 · 2 comments · Fixed by #1346
Closed

Update Navbar with options for logo widths and max width for mobile #1313

slowbot opened this issue Jun 3, 2021 · 2 comments · Fixed by #1346
Assignees

Comments

@slowbot
Copy link
Collaborator

slowbot commented Jun 3, 2021

Logo Width Proposal:

  • Add multiple width options using logo-slim, logo-med and logo-wide variants

Set Logo Max Width:

  • Add a max width to logo on mobile to avoid collision with menu button

Allow for Image only option:

  • Add a variant that hides text and displays logo only. Should still accept logo title but only accessible to screen readers
@akegan
Copy link
Contributor

akegan commented Jun 4, 2021

Thanks Jesse! I'd just add that we'd ideally be able to set this or default from the SiteHeader component. We're using that directly rather than pulling in a Navbar component directly.

I was able to get better behavior by calling SiteHeader with logoClass="normal", but even with that class, there's still an issue with overlap on small screens. It starts to become a problem around 370px

image
image

More context overall in this slack thread

@akegan
Copy link
Contributor

akegan commented Jun 4, 2021

@slowbot I wonder if it'd also make sense to expand this ticket to allow for the functionality of just having a logo lockup image with the text instead of providing the text separately?

It currently looks like this if you leave out the text.
image

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 a pull request may close this issue.

3 participants