Skip to content

Conversation

joedolson
Copy link
Member

Fixes #185

Copy link
Contributor

PR Preview
🚀 View PR Preview at https://wpaccessibility.org/pr-preview/pr-191.
If you see a 404 error, please wait a few minutes and refresh the page.
2025-09-30 20:52 UTC

Copy link
Member

@rianrietveld rianrietveld left a comment

Choose a reason for hiding this comment

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

@joedolson Yes, this adds color and brightens up the design!
A few things

  • In Firefox the logo is chopped off at the top
  • the menu text scrolls below the title,
  • on mobile view the logo is huge when zoomed in 200%, maybe adjust the size for smaller screen width

See screen shots

Firefox zoomed in 200% Safari small view port Firefox, logo chopped off at the top Firefox, menu scrolled down, menu text is mixing with the title text

@rianrietveld
Copy link
Member

Another thought: shouldn't the logo be clickable to the homepage, as in "expected behaviour"?

@joedolson
Copy link
Member Author

I debated over that, and tried it both ways. But the text on the image is significant, and I felt that needed to be reflected in the alt text, which wouldn't have been appropriate in a link.

I'm thinking you may not be getting the latest CSS in your rendering; the mobile logo should be considerably smaller than that.

@rianrietveld
Copy link
Member

@joedolson I did some more testing and this CSS solves the issues:

.site-header {
  display: flex;
  align-items: center;

  .lh-wrapper {
    align-items: center;
    img {
      max-width: 130px;
      padding-left: 1rem;
    }

    @include mq(lg) {
      img {
        max-width: 85%;
      }
    }

  }

}

The media queries are defined in: https://github.com/wpaccessibility/wp-a11y-docs/blob/main/_sass/abstracts/_media-query.scss

It also makes the logo less in your face and adds some padding and fixes the overlap of text in the sidedar menu.

screenshot of mobile view, small table and desktop of the homepage

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 this pull request may close these issues.

Add favicon/logo to site header
2 participants