Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Try using global site padding #88

Closed
wants to merge 4 commits into from
Closed

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Oct 12, 2021

This PR is an attempt to use the new global site padding theme.json entry as introduced in WordPress/gutenberg#35241.

For most templates, things look identical to what we had before:

Screen Shot 2021-10-12 at 8 00 48 AM

... however there's one major issue: the padding also applies to items that are set to full-width. This means we cannot have the full-width dark header:

Screen Shot 2021-10-12 at 7 56 36 AM

Similarly, full-width items in the post content do not extend to the edges of the screen (they don't do that with our current approach either). Not sure the best way around this — but I'd expect that if an item is specifically set to appear full-width, Gutenberg should let it be full-width?

@@ -1,5 +1,5 @@
<!-- wp:group {"style":{"elements":{"link":{"color":{"text":"var:preset|color|background"}}},"spacing":{"padding":{"top":"0px","right":"max(1.25rem, 5vw)","bottom":"max(1.25rem, 5vw)","left":"max(1.25rem, 5vw)"}}},"backgroundColor":"foreground","textColor":"background","layout":{"inherit":true}} -->
<div class="wp-block-group has-background-color has-foreground-background-color has-text-color has-background has-link-color" style="padding-top:0px;padding-right:max(1.25rem, 5vw);padding-bottom:max(1.25rem, 5vw);padding-left:max(1.25rem, 5vw)"><!-- wp:template-part {"slug":"header","tagName":"header","layout":{"inherit":true},"align":"wide"} /-->
<!-- wp:group {"style":{"elements":{"link":{"color":{"text":"var:preset|color|background"}}},"spacing":{"padding":{"top":"0px","bottom":"max(1.25rem, 5vw)"}}},"backgroundColor":"foreground","textColor":"background","layout":{"inherit":true}} -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also: I removed the left/right padding from here, but that defaults us to the default "Group block with background color" padding. It would be cool if that arbitrary value was automatically replaced with the global site padding value when one exists.

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 12, 2021

I had some followup discussions with @jasmussen about this, and it seems like solving for that full-width header while also using global site padding gets us into the weeds pretty quickly. Either we'd need to add some custom CSS with negative margins (I thought we'd left that world when we introduced Layout in Gutenberg! 😩), or we'd need to add CSS like that in Gutenberg to handle this there. For the latter though, it's difficult to figure out whether or not a user actually does want full-width items to go outside of the padding. After all, padding is generally meant to apply to all children.

So in general, since this theme relies on a core element (that homepage header) being full-width, I think this global site padding is probably not a tool that we should be using.

@jffng
Copy link
Collaborator

jffng commented Oct 12, 2021

I think this global site padding is probably not a tool that we should be using.

I agree, at least not in its current iteration. We need the ability for blocks at the root level (and ideally also blocks within pages and posts per #5) to actually be full width / extend to the edge of the page.

@jasmussen
Copy link

We need the ability for blocks at the root level (and ideally also blocks within pages and posts per #5) to actually be full width / extend to the edge of the page.

The block editor can absorb some complexity, and when we can, it's great. But ultimately we are bound by needing to output CSS in the end, and so zooming way out, the problem here remains one of deciding where padding is applied. If it's at the root level, how do we then stretch items over that padding? If we can build a nice codepen, we can work from there.

@jffng
Copy link
Collaborator

jffng commented Oct 12, 2021

how do we then stretch items over that padding?

What are your thoughts here? In Blockbase we use negative margins, and have had to refactor this bit many times to account for various use and edge cases. I'm happy to explore some other ideas.

@jasmussen
Copy link

My first instinct was negative margins as well. One challenge there is to "automate" them, as the negative margin has to match the positive padding. That can be done with a CSS variable and a little calc thrown in for good measure, but wiring that up to a good experience in global styles can be gnarly.

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 15, 2021

I'm going to close this one for now, while we discuss possible solutions in WordPress/gutenberg#35607.

@kjellr kjellr closed this Oct 15, 2021
@kjellr kjellr deleted the try/global-site-padding branch October 15, 2021 12:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants