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

Header & Content/Footer alignment doesn't match. #62

Closed
StevenDufresne opened this issue Nov 28, 2022 · 11 comments
Closed

Header & Content/Footer alignment doesn't match. #62

StevenDufresne opened this issue Nov 28, 2022 · 11 comments
Assignees
Milestone

Comments

@StevenDufresne
Copy link
Collaborator

StevenDufresne commented Nov 28, 2022

Footer & Content padding works the same. It uses clamp to provide responsive padding around the outer container. The header does not. The result is that the content does not match the header/subnav alignment.

Example

I'm not certain which way we should go since the header recently went through some changes in WordPress/wporg-mu-plugins#302.

@ryelle Thoughts?

@StevenDufresne StevenDufresne mentioned this issue Nov 28, 2022
8 tasks
@StevenDufresne StevenDufresne added this to the Initial Launch milestone Nov 28, 2022
@ryelle ryelle self-assigned this Nov 29, 2022
@StevenDufresne
Copy link
Collaborator Author

StevenDufresne commented Dec 1, 2022

@ryelle I see you made some changes in 2ca5e5b.

What else is needed for this?

@ryelle
Copy link
Contributor

ryelle commented Dec 1, 2022

What else is needed for this?

I'm not totally sure what else Showcase would need. In Documentation, I added a local override of the --wp--preset--spacing--60 variable, and then I'm using that (Large padding) on the left/right of my full-width containers — seems to be working out for that site. Maybe you can do something similar?

I added a note about moving that override back to the parent theme - if this works for you in Showcase too, we definitely should just update the value in wporg-parent-2021.

@StevenDufresne
Copy link
Collaborator Author

I think the problem is that the content quickly misaligns with the subnav as the viewport grows because it's responsive while the header and subnav are not.

Screen Shot 2022-12-02 at 8 12 03 PM

@StevenDufresne
Copy link
Collaborator Author

I spent a few moments investigating why.

The overrides for the sticky menu, designed to make it align with the header are fixed:
https://github.com/WordPress/wporg-parent-2021/blob/28da86bbd2babae96fd0ce6203ce9c9e894f0b5a/source/wp-content/themes/wporg-parent-2021/sass/block-styles.scss#L466
https://github.com/WordPress/wporg-parent-2021/blob/28da86bbd2babae96fd0ce6203ce9c9e894f0b5a/source/wp-content/themes/wporg-parent-2021/sass/block-styles.scss#L478

In order to have the header, content, and footer aligned. We'll need to adopt those properties and remove the clamping by overriding it (--wp--preset--spacing--60). We need to override it for the body because the footer also uses --wp--preset--spacing--60 and if we don't the content will match the header but not the footer. I think we can do it up until the content max width.... I think. I'll try a PR.

@StevenDufresne
Copy link
Collaborator Author

After giving this a try, I think we should just remove responsive clamping on all outer containers to match the header (content & footer) for wp.org.

Does anyone have any thoughts on this @ryelle @adamwoodnz @renintw ?

@adamwoodnz
Copy link
Contributor

After giving this a try

Would you be able to push a branch with this? I'll find it easier to provide input after testing I think

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Dec 7, 2022

Just had a quick look. If we do adopt the header padding for the content, it seems the one format which might not have a desirable layout is tablet portrait, where the central space (gap) will be considerably larger than the edge space. The edge space will be quite tight.

Tablet landscape Tablet portrait
wordpress org_showcase-test_archives_(iPad Air) (1) wordpress org_showcase-test_archives_(iPad Air)

I actually don't mind the misalignment in portrait as it gives some breathing room to the sites. I think it looks odd in landscape though. Feels like above a certain width ( >= tablet landscape?) the header should align with the content. Below that it should align with the main header.

@ryelle
Copy link
Contributor

ryelle commented Dec 7, 2022

I think the problem is that the content quickly misaligns with the subnav as the viewport grows because it's responsive while the header and subnav are not.

Header, subnav, and (as of WordPress/wporg-mu-plugins@2ca5e5b) the footer, are all responsive, it's just a cliff 80/20px rather than the scaled clamp.

We'll need to adopt those properties and remove the clamping by overriding it (--wp--preset--spacing--60).

Yes, that's what I was trying to suggest in #62 (comment) by showing the CSS code from documentation. Then in the templates, whatever top-level full-width container you have should get left & right padding of --wp--preset--spacing--60, for example, this in archive.html.

Once things have calmed down a little, we can look at whether --wp--preset--spacing--60 should be updated in the parent, or whether this should actually be a separate custom property.

@StevenDufresne
Copy link
Collaborator Author

Ok. I think we can launch with the layout as is. I'll update the milestone to reflect that.

@ryelle
Copy link
Contributor

ryelle commented Dec 15, 2022

I've set up an issue to track fixing this in all the child themes, since if we make the change in the parent theme first, it could break alignment in the child themes.

@ryelle
Copy link
Contributor

ryelle commented Aug 1, 2023

Closing this, if it's still valid it'll be fixed with #130.

@ryelle ryelle closed this as completed Aug 1, 2023
@ryelle ryelle closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants