Background image: should it integrate with background-image theme mod?

ramonjd commented 6 months ago

What's the issue?

Site background images in WordPress pre-block editor

in this issue a "site background image" refer to an image set via the background-image CSS property to the body tag.

WordPress themes have long been able to set site background images and other background properties such as size and repeat (on the HTML body element) either by the Customizer UI or, more directly, using custom-background theme support. For example:

add_theme_support(
    'custom-background',
    array(
        // As well as default image, there's default-position-x, default-position-y, default-size, default-repeat, default-attachment
        'default-image' => get_stylesheet_directory_uri() . '/img/bird.jpg',
    )
);

Or like this:

add_theme_support( 'custom-background' );
// As well as background_image, there's background_preset, background_size, background_position_x, background_position_y
set_theme_mod( 'background_image',  get_stylesheet_directory_uri() . '/img/bird.jpg' );

Enabling background support adds a link to the Customizer in the admin menu. Users can add/edit the background image using Customizer controls:

Link in sidebar	Customizer controls

Site background images in block themes/block editor

Since https://github.com/WordPress/gutenberg/pull/59354 and https://github.com/WordPress/gutenberg/pull/59454, users can add site background images via theme.json or global styles in the site editor.

However, if a site background image is set via theme support/mods, it will overwrite any theme.json or global styles image set in the site editor because of CSS specificity, that is body.custom-background will trump :where(body).

What is the question? Or your proposed solution?

The main question is whether Gutenberg should take values set by theme support/mods into account by:

using any background- values from theme support/mods as defaults for top-level background property values.
overwriting theme support/mod values with theme.json/global styles values, perhaps via a higher CSS specificity or preventing WordPress from outputting the custom-background-css style tag
??

ramonjd commented 6 months ago

preventing WordPress from outputting the custom-background-css style tag

This will work I think:

add_filter( 'current_theme_supports-custom-background', function() {
    return false;
} );

carolinan commented 6 months ago

Pinging @WordPress/outreach @WordPress/block-themers for feedback.

I would expect it to work like the logo block:

Setting the logo using the block updates the theme mod.
Setting the logo in the customizer updates the block.
When a user saves the setting, neither defaults set by the theme are used, the value in the database is used instead.

But the site logo block can not be set using theme.json. It would be good to make sure that these two blocks work the same way.

It may be too late now but I would have expected the new option to also output and be applied to the custom-background class.

overwriting theme support/mod values with theme.json/global styles values,

I think this would be a "user error"... I am not sure why a theme developer would add conflicting values in the two settings, unless by mistake? Maybe I am underestimating how many existing classic themes there are, that are being converted to block themes. Perhaps as a theme developer, I would make sure to: Get the theme mod. Programatically add it to theme.json or use a filter. Remove the theme mod.

ramonjd commented 6 months ago

Thanks a lot, @carolinan!

overwriting theme support/mod values with theme.json/global styles values,

I think this would be a "user error"... I am not sure why a theme developer would add conflicting values in the two settings, unless by mistake?

Yes, I agree about the "user error" point when it comes to theme.json.

I think I was mainly worried about background images set via global styles in the site editor. So, if a theme sets an image using the custom background theme mod, should a user be able to overwrite that in the site editor?

It may be too late now but I would have expected the new option to also output and be applied to the custom-background class.

Okay, so one option would be for Gutenberg to "sync" site background image values in global styles with the existing custom-background options (if activated).

I'm thinking out loud here.

The two storage mechanisms would co-exist (top-level global styles and theme mod options) in the following scenarios:

'custom-background' theme mode activated

If a background image properties are set in theme.json (user error), one needs to take precedence. I'd say the theme.json values should prevail given that theme.json is the "portable" component.
If no background image properties are set in theme.json, Gutenberg would use theme mode options to populate top-level background image global style values in the background. Not sure where this should happen maybe using the wp_theme_json_data_theme filter and get_theme_mod( PROPERTY, get_theme_support( 'custom-background', PROPERTY ) ) values.
Whenever top-level background image global styles are updated in the site editor, Gutenberg would also update the corresponding theme mode options, probably via set_theme_mod as it's done in WordPress.
Whenever theme mode options are updated via the Customizer, Gutenberg would also update the corresponding theme mode options possibly via pre_set_thememod{$name}

'custom-background' theme mode NOT activated

Use global styles as per normal.

The above would be a large task to implement.

The alternative, as mentioned in the description might be to have two, separate storage systems - theme.json/global styles and theme mods options, with the former taking precedence always.

carolinan commented 6 months ago

if a theme sets an image using the custom background theme mod, should a user be able to overwrite that in the site editor?

If the user has any access to setting the background in the Site Editor, yes.

markhowellsmead commented 6 months ago

The specificity of inline CSS has been a problem for a long time. (https://github.com/WordPress/gutenberg/issues/40159) I realise that the assignment of a user-selected background image has to be achieved in this way, but the overridable specificity of inline CSS remains critical and, often, unsolved.

ramonjd commented 6 months ago

I realise that the assignment of a user-selected background image has to be achieved in this way, but the overridable specificity of inline CSS remains critical and, often, unsolved.

Yeah, it's something that probably has to be resolved in some way. I'm very empathetic to this view point! 😄 especially since I've been trying to dismantle Guteberg's curious use of CSS background for the color gradient. 🙃

If I've not misunderstood your comment, for this issue specifically, it's dealing with top-level styles and CSS rules that are written in a stylesheet using selectors, not inline CSS on elements.

markhowellsmead commented 6 months ago

I don't want to divert the topic of this ticket. The relevance is that the specificity of the selector body.custom-background is too high, which exacerbates the problem that placing it inline makes it very difficult indeed to override.

carolinan commented 6 months ago

While to me it is logical that the body tag has a custom-background class so that I can programatically check if the current page has a custom background.

markhowellsmead commented 6 months ago

I completely agree, Carolina. However, the CSS selector needs to be less specific. I would imagine that wrapping it in :where would quite possibly resolve that problem.

markhowellsmead commented 6 months ago

I'd also add that the priority should be theme.json » customiser, and that the latter value should be applied by adding it later in the cascade: not by increasing the specificity of the CSS selector.

ramonjd commented 4 months ago

Notes on syncing the Customizer and theme.json values after chatting with @aaronrobertshaw

Classic themes with theme.json

Any background image properties set in theme.json should be reflected in the background image Customizer settings.

Subsequent changes in the Customizer always take precedence. When the Customizer value is cleared, however, should the theme.json default values kick in?

What if the user wants to remove the background entirely?

Block themes

Any background image properties set in theme.json or updated in global styles should be reflected in the background image Customizer settings.

Changes to one with affect the other. For example, if a user updates the background image settings in the Customizer it should be saved in the user global style custom post.

It's a bit of an edge case that both systems will be in use in block themes though.

Other sources

Plugins or themes can set background image values programmatically via add_theme_support.

How should WordPress honour these values? If at all?

Let's say a theme has a site-wide background image defined in theme.json, and it also adds one using add_theme_support. It's another edge case, and probably user error, but the theme.json/global styles values should prevail.

WordPress / gutenberg