Global styles: force root min-height of 100% for backgrounds

[ramonjd]

What?

Initial commit - set html min-height to 100% for backgrounds (background or background-image properties)

See: #59354 (comment)

Why?

The objective is to ensure that the :html selector has a minimum height of 100% when:

1. There's a background gradient (styles.color.background)
2. There's a background image (styles.background.backgroundImage)

The reason is so the background gradient or image takes up the entire screen.

The block editor already does this by giving the HTML tag a min-height of 100% all the time, so the corollary objective is to have the frontend match this.

How?

Not sure yet 😄

So far, just detecting the background and hardcoding html { min-height: calc(100% - var(--wp-admin--admin-bar--height, 0px)); }

Testing Instructions

In the site editor or in theme.json, add a gradient background to your site:

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"settings": {
		"appearanceTools": true
	},
	"styles": {
		"color": {
			"gradient": "linear-gradient(#e66465, #9198e5)"
		}
	}
}

Also try with a background image:

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"settings": {
		"appearanceTools": true
	},
	"styles": {
		"background": {
			"backgroundImage": {
				"url": "https://i0.wp.com/wordpress.org/files/2023/12/sotw-dotorg-drawer.png?w=1807&ssl=1",
				"source": "file"
			},
			"backgroundSize": "cover"
		}
	}
}

Screenshots or screencast

Before	After

[ramonjd]

@andrewserong @tellthemachines

Before I go too deep on this PR, I wanted to garner a few opinions on how we might achieve the objectives.

The PR's description outlines the why of this PR, but the technical criteria is that, when there's a gradient background or background size of cover at the root level, we want to force the window (HTML or :root) to have a min-height: 100% so the background stretches the full height without scrolling.

This is a naive attempt to add the requisite CSS.

The first, hopeful, attempt was to switch the static::ROOT_BLOCK_SELECTOR from 'body' to :root/html, and then set the value of styles.dimensions.minHeight. This would have been my preferred approach, however, it came with a bunch of unknowns, one of the weirdest being that it doesn't allow themes to overwrite a browser's default margins.

Then I mused whether html could be a top level "element", and therefore be "styleable", but I abandoned that idea. It might have legs, I don't know, but it'd be a bunch of work.

Then I landed here, at the MVP.

It works, but I would be glad of alternative points of view 😄

Some considerations:

• How, if at all, could a theme developer overwrite the min-height value? Should they at all? Maybe not since it's tightly coupled to the background option. Or am I thinking to hard about it? 😄
• I plan to abstract the logic - if you can see avenues for optimization, that'd be great!

[andrewserong]

Just some quick thoughts off the top of my head:

• I think it's fine if the output is :root but that the ROOT_BLOCK_SELECTOR is still body as the rules here can be different to the selector since it's a slightly non-standard use case. The important thing IMO is that we're outputting root rules of some kind just a single time. Out of curiosity, why do we need the rule to be attached to :root instead of body? Is it because of potential margins on body?
• Should there be a way for folks to explicitly switch this setting off? I do like that it's guarded behind only being output when background or backgroundImage is set at the root level 👍.
• Since you asked the question of whether a theme developer could overwrite the min-height value, I think I can imagine someone wanting to do just that! I don't think it'd necessarily need to be implemented in this PR, but if a theme were to set the root styles.dimensions.minHeight value in their theme.json file, perhaps that could overwrite the value used here? In that case, though, I imagine the theme developers would expect the value to always be output, not just when background is set 🤔
• In terms of abstraction, what did you have in mind? If it's only being used in one place, I like the idea of it essentially being private code so that it could be easier to change in follow-ups, potentially.

Thanks for digging in here!

[tellthemachines]

How, if at all, could a theme developer overwrite the min-height value? Should they at all?

Is there a need for overriding this value? It's an essential part of making sure gradients and background images work correctly on the site background.

A possible alternative would be setting min-height to a custom property via the background block support (so we'd only output it if there were a background gradient/image), and then we'd only have to set the custom property's value in this function when static::ROOT_BLOCK_SELECTOR === $selector. However, that would mean outputting min-height: var(--something) whenever there were a background set. Would we ever want to set min-height together with background in other circumstances?

[tellthemachines]

On the topic of abstraction, I'm not entirely sure about the reasoning behind static::update_separator_declarations but it looks like something that should be solved in the block itself by skipping serialization and adding custom logic to deal with it 🤔 It would be great to reduce the amount of functions in the theme.json class if at all possible.

[ramonjd]

Thanks a lot for the helpful feedback! I feel like I have some paths forward, now 🙇🏻

Out of curiosity, why do we need the rule to be attached to :root instead of body? Is it because of potential margins on body?

Good question! Two reasons:

1. min-height: 100% on the body tag doesn't have any effect on when using background gradients. min-height: 100vh, however, does. See: https://codepen.io/BoganJustice/pen/jORMKKE
2. I couldn't quite get the scrolling to behave with adding it to the body tag because of various margins of nested elements. The video below is testing 100vh but it's also the same for 100%

2024-03-15.10.06.13.mp4

I haven't reached the end of experimentation, yet though. If it's just the logged-in admin view, then it might be something we can deal with.

Also, I suspect min-height: 100vh; on the body takes care of most situations, in which case it would fit nicely with theme.json styles.dimensions.minHeight.

Should there be a way for folks to explicitly switch this setting off? I do like that it's guarded behind only being output when background or backgroundImage is set at the root level

If I can get it to play with styles.dimensions.minHeight then maybe it'll be already taken care of.

Is there a need for overriding this value? It's an essential part of making sure gradients and background images work correctly on the site background.

You're right. It must be 100% for it to have the intended effect on the background. I like the idea of creating a CSS custom property to be more flexible - that could potentially be overridable.

But yeah, it might be good enough to start with the CSS rule and then decide before creating a global CSS property that folks might rely on.

If it's only being used in one place, I like the idea of it essentially being private code so that it could be easier to change in follow-ups, potentially
It would be great to reduce the amount of functions in the theme.json class if at all possible.

Good points. I'll leave the code in place for now. I'm not excited about another for loop there, but I can't see a tidy alternative.

[tellthemachines]

I like the idea of creating a CSS custom property to be more flexible

The custom property wasn't for flexibility, it was a way to avoid the for loop here 😅 .
The idea is you'd set min-height: var(--something) in the background block support, whenever you're setting gradients or background images. But the property itself isn't set unless the declarations are being applied to the root level. You'd probably need something like `html { --something: 100%; } body { --something: unset } so that value doesn't accidentally apply to all background supports. It's messy though, unless there's a use for min-height in non-root cases.

[ramonjd]

Why on the HTML and not BODY?

• setting it on the body revealed potential issues, e.g., the background gradient did not always stretch (unless we used 100vh)
• Also unpredictable margins causing scrolling , e.g., an unstyled H1 at the root of the document in empty theme
• Settings "styles.dimensions.minHeight" in theme.json would overwrite the value - I suppose this would be fine for theme devs who know what they're doing, but for folks using themes that have "styles.dimensions.minHeight" hardcoded it would mean that backgrounds don't behave as they should, that is, with the background gradient/image filling the window.
• 'HTML' is also the selector that's being used in the editor to achieve the same effect.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[andrewserong]

Just wondering, would we also want the min-height rule to be output if we use contain as the background size? I.e. should we always output the min height if any background image is set, a little like how we're outputting if any background shorthand property is in use? 🤔

[ramonjd]

Just wondering, would we also want the min-height rule to be output if we use contain as the background size?

Good question. I don't know, but I'll test it out.

I.e. should we always output the min height if any background image is set, a little like how we're outputting if any background shorthand property is in use?

We could I suppose, so long as it behaves as expected with other possible background property values. I'll also play around with it.

[ramonjd]

Just wondering, would we also want the min-height rule to be output if we use contain as the background size?
I.e. should we always output the min height if any background image is set, a little like how we're outputting if any background shorthand property is in use?

In the case of "contain", it kinda depends on the image size I think. All other values, aside from "cover", the min-height has no effect as far as I can see.

background-size: contain

800px - with min-height 100%	800px - without

2800px - with min-height 100%	2800px - without

background-size: auto / background-size: unset / no background-size

with min-height 100%	without

As with auto above, any other value, e.g., 40%, 100px 400px and so on has no affect.

Given that the site editor has a permanent min-height: 100%, I think there's a justification to always output it.

Maybe later we can decide whether to provide a way for users to control the value or toggle it? 🤔

[andrewserong]

In the case of "contain", it kinda depends on the image size I think. All other values, aside from "cover", the min-height has no effect as far as I can see.

Nicely captured, thanks for confirming!

Maybe later we can decide whether to provide a way for users to control the value or toggle it? 🤔

Sounds reasonable to me 👍

[andrewserong]

This is testing very nicely for me, and feels safely guarded in that it'll only be output if folks are using a site-wide gradient or background image, so the chances for any negative impact (i.e. sites out in the wild with a site-wide gradient) seem very minimal to me, and most likely it'll feel more like a bug fix 👍

Also, as you mention, we can always explore allowing the value to be customised in follow-ups if need be.

Looking good with site-wide background images in local testing:

Before	After

LGTM! ✨

[ramonjd]

Thanks for all the help testing these @andrewserong 🙇🏻