Header template part cleanup

[MaggieCabrera]

Description

This PR removes all extra headers and reduces it to just one. I've replaced the glyph with the site logo.

Closes #56
Closes #311

Screenshots

@beafialho I have seen that there's just one header in the figma, but the old demo sites were using different ones to display the different glyphs, I think I made the correct assumption removing the extra template parts and keeping just one. Some places the header is sticky, some places it isn't. The same with the border bottom. We can express this by making changes to the templates themselves if needed, I just need to know when we want either of those to happen, can you let me know? I will update this PR with that info.

[beafialho]

@MaggieCabrera let's make the header sticky everywhere, and let's not use the border anywhere.

One minor adjustment I ask you to do is change the a background color of the header group to #FAFAFAF5 instead of #FFFFFF, which gives it the same color as the site background but with a nice transparency. Do you think we should add it to the color palette, for this to work well with the Style Variations?

Finally, is it possible to specify a maximum dimension to the Site Logo?

[MaggieCabrera]

One minor adjustment I ask you to do is change the a background color of the header group to #FAFAFAF5 instead of #FFFFFF, which gives it the same color as the site background but with a nice transparency. Do you think we should add it to the color palette, for this to work well with the Style Variations?

Where else would we be using that color? And what would be the alternative for that color in those style variations?

Finally, is it possible to specify a maximum dimension to the Site Logo?

I think we can define the width only

[beafialho]

Where else would we be using that color? And what would be the alternative for that color in those style variations?

Nothing I remember. In the style variations, the shade could be #212121ED. Here's the expected view:

Unless we could set the respective background colors but specify the opacity separately or elsewhere?

I think we can define the width only

Hmm, can we define the aspect ratio? I'd rather not specify width because for squared logos, a small width would work well but for long logos, it would not 🤔

[MaggieCabrera]

Ok, I'll add it to the theme.json, else I don't think we can make it work with the variations.

And no, sadly the site-logo doesn't support aspect ratio that I know of

[MaggieCabrera]

WordPress/gutenberg#52169 the issue for aspect ratio on site logo

[beafialho]

Ok, let's leave the site logo as is then. Thank you!

[MaggieCabrera]

The "stickyness" wasn't working when applied inside the template part, so I used a wrapper and applied the header tag to it instead. I also added the extra color, let me know if you think the naming convention is not correct @richtabor

/cc @carolinan for markup review too

[richtabor]

@beafialho I think we're going to get tagged via accessibility with the transparent header background. I suggest we revert it back to the base color.

[beafialho]

I think it could be worth giving it a try and revert if it's not accepted 🙂 the transparency is so small, but the detail really makes a difference.

[MaggieCabrera]

@joedolson can you chime in on this, please? I'm happy to revert if needed

[joedolson]

Transparency isn't fundamentally a problem; it becomes a problem if it creates a contrast issue that makes the foreground content (the header) unreadable when the background is behind it. The transparency here is subtle enough that I don't see a problem.

I am noticing that the header has two header elements, however, and that's a problem; it causes screen readers to double announce the banner landmark region. There should only be one header element; the other element should just be a div.

[richtabor]

Transparency isn't fundamentally a problem; it becomes a problem if it creates a contrast issue that makes the foreground content (the header) unreadable when the background is behind it.

Agreed — but I'm not entirely confident in these results:

If we added backdrop blurring, that would help, but I don't think we should add that level of custom CSS— which can't be manipulated outside of code.

[joedolson]

There aren't any problems explicitly in those examples. That's not to say it's ideal - adding extra lines and angles, however subtle, behind written text is going to make it harder to read for somebody. However, with these colors, it doesn't cause a specific WCAG violation.

If a user changed the colors, however, so that the header text was a gray that just barely passed accessibility against white, then it would become a problem.

[richtabor]

Thanks @joedolson.

@beafialho I think we should play it safe and not do the transparent header. I agree it's a nice touch, though it's too fragile where folks will end up with a less than ideal result.

[richtabor]

I think I made the correct assumption removing the extra template parts and keeping just one.

Just thinking aloud here:

If we don't include the template parts for each, we will be unable to place different header/footer parts in different templates.

But maybe that's not a bad thing.

If you replace a template with another, and you get a different header, your whole site will have header A—while that template has header B. Then you have to figure out how to edit both. If there's one header template part used throughout, you can replace them with another and it happens site-wide.

[beafialho]

Thanks @joedolson.

@beafialho I think we should play it safe and not do the transparent header. I agree it's a nice touch, though it's too fragile where folks will end up with a less than ideal result.

Ok, sounds good.

[MaggieCabrera]

I am noticing that the header has two header elements, however, and that's a problem; it causes screen readers to double announce the banner landmark region. There should only be one header element; the other element should just be a div.

I think this has been fixed, where do you see it?

[joedolson]

In the rendered source, with this PR applied:

Image shows nested headers: .wp-block-group .is-position-sticky and .wp-block-template-part.

[richtabor]

Image shows nested headers: .wp-block-group .is-position-sticky and .wp-block-template-part.

Yes, this needs to be resolved.

[richtabor]

I think this is a decent first pass. May end up adjusting it a bit further, but let's get this in before too many conflicts.

[MaggieCabrera]

Mmmh, I didn't know the header template part defaulted to using the header tag, my intention was to have the sticky wrapper be the one with the header tag, so it's first in the DOM, but I'm not sure how important that is. If need be, we can iterate on that.

[richtabor]

Mmmh, I didn't know the header template part defaulted to using the header tag

Yea, that's what I did the opposite first. It's important to keep them "header" parts, for the replace flow and organization in the UI.

[MaggieCabrera]

Mmmh, I didn't know the header template part defaulted to using the header tag

Yea, that's what I did the opposite first. It's important to keep them "header" parts, for the replace flow and organization in the UI.

very right! thanks for pointing that out