-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Global styles: add background image to top-level theme.json styles #59354
Conversation
a230e0f
to
841e782
Compare
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/block-supports/background.php ❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/block-supports/background-test.php ❔ phpunit/class-wp-theme-json-test.php |
Size Change: +35 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
packages/blocks/src/api/constants.js
Outdated
backgroundImage: { | ||
value: [ 'background', 'backgroundImage' ], | ||
support: [ 'background', 'backgroundImage' ], | ||
useEngine: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly temporary, possibly not.
Bypass style engine because it's expecting url
and source
properties.
if ( _backgroundImage?.source === 'file' && _backgroundImage?.url ) { |
But I'm not yet convinced these need to be replicated in theme.json, which should support at first an image path string value for background.backgroundImage
(?), and potentially ref paths in blocks, e.g., "backgroundImage": { "ref": "styles.background.backgroundImage" }
Options:
- Convert
background.backgroundImage (string)
toarray()
withurl
andsource
properties in the backend after processing theme.json - Imply in all contexts that URL-like strings coming from
styles.background.backgroundImage
are equal to{ source: 'file', url: styles.background.backgroundImage }
- ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might want to support an object for background.backgroundImage
in each context (block attributes and theme.json) in order to allow source
to be set to things other than file
(e.g. featuredImage
, theme
if we want to prepend the theme's directory to the URL, etc). But, for ease of use, it'd likely be good have it support a simple string, too.
How bad would it be to allow the property to be a string or an object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source to be set to things other than file (e.g. featuredImage, theme if we want to prepend the theme's directory to the URL, etc)
Thanks for the explainer @andrewserong I couldn't find any background on why source
was there.
I suppose having source
as a context marker would be handy to add extra functionality.
How bad would it be to allow the property to be a string or an object?
In that case, my opinion is that we could support both in theme.json.
Where "backgroundImage": "http://some_url/image.gif"
is the equivalent of { source: 'file', url: 'http://some_url/image.gif' }
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where "backgroundImage": "http://some_url/image.gif" is the equivalent of { source: 'file', url: 'http://some_url/image.gif' }
Sounds great to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... the style engine expects string values and does nothing with them
I just realized this is a good idea in case users want something like:
background-image: linear-gradient(
to bottom,
rgb(255 255 0 / 50%),
rgb(0 0 255 / 50%)
), url("catfront.png");
Example taken from https://developer.mozilla.org/en-US/docs/Web/CSS/background-image
I've updated the branch to wave strings through like this.
8dbde9c
to
0514a4c
Compare
lib/block-supports/background.php
Outdated
|
||
/* | ||
* @TODO document | ||
* $background_styles['backgroundImage']['url'] and file source implies an image URL path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be documented. The logic, I believe, is as follows:
- backgroundImage is a string: We accept whatever the value is. It could be
url(...)
or some combination thereof. The style engine will sanitize. - backgroundImage is an object with url and source props: it will be processed by Gutenberg, in the case of "file" or "theme" wrapped in a
url()
function and escaped. CSS defaults likecover
will kick in.
Updating constants and docs Fix props
… background and global styles
Added tests Supporting string and object for backgroundImage
…viour Now assuming that a string value will have it's own `url()`
Note to self: I was playing around with patterns and templates, where we specify relative paths + Here's an example pattern: <?php
/**
* Title: Empty theme pattern
* Slug: emptytheme/empty-pattern
* Categories: featured
*/
?>
<!-- wp:group {"style":{"background":{"backgroundImage":{"url":"/img/NYPizzaPie.jpg","id":9,"source":"theme","title":"NYPizzaPie"},"backgroundPosition":"28.000000000000004% 37%","backgroundSize":"cover"},"dimensions":{"minHeight":"299px"}},"layout":{"type":"constrained"}} -->
<div class="wp-block-group" style="min-height:299px"></div>
<!-- /wp:group --> It could also be HTML. It works fine on the frontend given that the block supports functions render the image. In the editor it's a bit trickier: there's no equivalent of Perhaps they can be parsed when the patterns are registered 🤷🏻 Anyway, I'm just jotting this down so we can be aware of it in follow up. I think this PR shouldn't support "theme" just yet, but its commits demonstrate that it's possible and the |
d4bd5ec
to
6bfcb74
Compare
Confirmed. A If we remove this rule then the frontend and editor background behaviour matches more closely. @t-hamano What do you think about removing that Even with that rule in the editor, the behaviour is still apparent on the frontend I guess the real question is: how closely do we want the site editor and frontend to match? |
Nice sleuthing!
I wonder if there's an opportunity for a feature of backgrounds (image or gradient) for folks to be able to set this in some way? (either on or off?) Or is that too "advanced settings" as an idea 🤔 |
Yeah, good idea. I suppose it might sit under Dimensions at the top level? Not sure if it's an intuitive control. I guess an alternative might be to add the same style to the frontend when there's a gradient background or background image. 🤷🏻 |
Yeah, I'd imagine it might be something that's sort of set implicitly, but that folks could then control if they really needed to (or adjust in
It is a tricky one — IMO if we're outputting styles on the site frontend, then there'll likely be someone who'd like to have control over it somewhere. If we need to output a min height, we could always allow |
Thanks! Agree, that is an approach definitely worth considering. It'd have to be set on |
Just one more question 😄 I noticed there's an item on the Tracking issue:
Was the intention here to move gradient backgrounds over to "background"? |
That line wasn't so much about the UI as about the fact that at the moment, gradient backgrounds use the Eventually, it'd be cool to allow layered backgrounds with images and gradients, and potentially multiple images, etc. That item was sort of a placeholder for any and all of the above ideas 🙂 Separately, though, it'd be cool to explore ideas with designers for how the controls could potentially be combined? |
Sorry for the late reply!
If a gradient is applied to the background, I would expect the gradient not to repeat. That's why I submitted #51374.
{
"$schema": "https://schemas.wp.org/trunk/theme.json",
"version": 2,
"styles": {
"color": {
"background": "tomato"
},
"dimensions": {
"minHeight": "calc(100vh - var(--wp-admin--admin-bar--height))"
},
"background": {
"backgroundImage": "url('https://i0.wp.com/wordpress.org/files/2023/12/sotw-dotorg-drawer.png?w=1807&ssl=1')",
"backgroundPosition": "center center",
"backgroundRepeat": "no-repeat",
"backgroundSize": "cover"
}
}
} |
@t-hamano thanks for looking into it! I think it's worth an experiment 😄 |
Just jotting down some notes...
In the frontend logged-in view (with admin bar), to avoid scrolling, it looks like we'd also have to account for any other dimensions on the root tags. E.g., It's a bit of voodoo to get it perfect, e.g,. tweaking reveals that Maybe it's not that important.
Seems like a useful property to allow in theme.json, getting Regardless, to dynamically add I'll add this task to the tracking issue regardless. Thanks for the help, folks! |
Dev NoteIn WordPress 6.6 you can define site-wide background images in theme.json and the Site Editor. A "site-wide" background image is one whose value is set on the body element using the An example might be a photo that stretches with the window size, or a repeating pattern background. To customize how background images appear, WordPress 6.6 supports the following background style properties:
UsageIn theme.jsonIn theme.json, site-wide background images and their properties are defined under For example, as a single image URI in {
"styles": {
"background": {
"backgroundImage": {
"url": "http://path.to/some/image.png"
},
"backgroundSize": "cover"
}
}
} For the above, WordPress will automatically wrap the image in the CSS
{
"styles": {
"background": {
"backgroundImage": "url(http://path.to/some/image.png)",
"backgroundPosition": "center"
}
}
} The above examples use absolute paths to image files. Such files would need to be hosted and maintained. Most likely, theme developers will want to define background images using paths to a theme's own assets. This ensures that the theme is self-contained and portable. Relative paths to theme assets are defined using the {
"styles": {
"background": {
"backgroundImage": {
"url": "file:./assets/my-theme-background.jpg"
},
"backgroundSize": "cover"
}
}
} Paths are resolved on the backend using get_theme_file_uri. Paths defined this way must be relative to the theme root, regardless of where the theme.json sits in your theme's directory. This follows an existing pattern for web fonts. Despite the dot in An issue exists to make the syntax more consistent. In the Site EditorBackground images can be also be uploaded, and their properties tweaked through the Site Editor's styles panel. In WordPress 6.6, background image controls are located under Styles > Layout. The styles panel navigation is undergoing review however, so in upcoming versions the location may change. As well as setting new background images, it's possible to "unset" a theme's default background image in the Site Editor. Relative paths to any images in theme.json are resolved on the backend, and are sent in the LimitationsIn WordPress 6.6, the ability to define background images in theme.json exists only for top-level styles. Top-level styles apply to the body element. An open PR aims to also enable the feature at the block level in the next WordPress release. Work is also underway to:
Backwards compatibilityWordPress already has support for custom site-wide background images in the Customizer. Background images added in the Customizer take precedence over those set in theme.json or in the Site Editor. An ongoing discussion seeks to harmonize the two features. What's nextProgress on upcoming work is tracked on Background Image block support follow-up tasks |
Hi there @ramonjd The dev note looks good. Do you have bandwidth to post it in draft on the Make Blog for first and second review? cc @juanmaguitar |
Yes definitely. I'll post the review link on the 6.6 Dev note tracking issue soon. Thank you! |
Dev Note 📓
#59354 (comment)
What?
Part of:
Enabling background image block supports at the root level of global styles.
TODO
get_theme_file_uri( $path )
?How?
Add
style.background
properties as valid top-level styles in theme.json.Testing Instructions
Check this branch out and enable emptytheme for ease of testing.
Below are some test theme.jsons.
Ensure that the
background-*
CSS appears in the body selector and that everything looks fine.Testing Instructions for Keyboard
Screenshots or screencast