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

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

Open
ramonjd opened this issue Apr 22, 2024 · 11 comments
Open

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

ramonjd opened this issue Apr 22, 2024 · 11 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Question Questions about the design or development of the editor.

Comments

@ramonjd
Copy link
Member

ramonjd commented Apr 22, 2024

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
Screenshot 2024-04-22 at 1 44 09 pm Screenshot 2024-04-22 at 1 43 36 pm

Site background images in block themes/block editor

Since #59354 and #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).

Screenshot 2024-04-22 at 11 50 44 am

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
  • ??

Related discussion: #60578 (comment)

@ramonjd ramonjd added [Type] Question Questions about the design or development of the editor. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Apr 22, 2024
@ramonjd
Copy link
Member Author

ramonjd commented Apr 22, 2024

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
Copy link
Contributor

carolinan commented Apr 23, 2024

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
Copy link
Member Author

ramonjd commented Apr 23, 2024

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

  1. 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.
  2. 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.
  3. 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.
  4. Whenever theme mode options are updated via the Customizer, Gutenberg would also update the corresponding theme mode options possibly via pre_set_theme_mod_{$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
Copy link
Contributor

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
Copy link

The specificity of inline CSS has been a problem for a long time. (#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
Copy link
Member Author

ramonjd commented Apr 24, 2024

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
Copy link

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
Copy link
Contributor

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
Copy link

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
Copy link

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
Copy link
Member Author

ramonjd commented Jul 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Question Questions about the design or development of the editor.
3 participants