WIP Experiment: combine gradient and background image into background-image
#60739
Conversation
|
Size Change: +302 B (+0.02%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
| 'background-color' => array( 'color', 'background' ), | ||
| 'background-image' => array( 'background', 'backgroundImage' ), | ||
| 'background-image' => array( | ||
| array( 'color', 'gradient' ), |
There was a problem hiding this comment.
Order matters here, determining the order of values within the property, e.g.,
background-image: linear-gradient(180deg, rgba(217, 97, 61, 0.69) 0%, rgba(250, 250, 250, 0.24) 100%), url(http://localhost:8888/wp-content/uploads/2024/04/Westwood-Pizza-newtown-1.jpg);The background image doesn't yet officially support opacity but the gradient does, so I think it's fine to keep this order for now.
Let's assume the background image does support opacity for a minute: the order would have to be specified somehow, perhaps by an index or foreground/background property in backgroundImage?
Thinking further ahead, background-image also supports multiple images. The order in which they appear in theme.json or via some sorting UI control should suffice here.
ramonjd
added
[Type] Experimental
| @@ -124,7 +124,9 @@ protected static function filter_declaration( $property, $value, $spacer = '' ) | |||
| $filtered_value = wp_strip_all_tags( $value, true ); | |||
|
|
|||
| if ( '' !== $filtered_value ) { | |||
| return safecss_filter_attr( "{$property}:{$spacer}{$filtered_value}" ); | |||
| // @TODO safecss_filter_attr can't handle background-image with multiple values. | |||
There was a problem hiding this comment.
Need a core patch to test that the following background-image CSS is allowed by safecss_filter_attr:
- Linear gradient + image:
background-image: linear-gradient(rgba(0, 0, 255, 0.5), rgba(255, 255, 0, 0.5)), url("https://domain/some/image.png"); - The reverse:
background-image: url("https://domain/some/image.png"), linear-gradient(rgba(0, 0, 255, 0.5), rgba(255, 255, 0, 0.5)); - Multiple images: background-image: url("some/image.png"), url("https://domain/some/other/image.png");
- Any other valid values, e.g.,
background-image: linear-gradient(
to bottom,
rgb(255 255 0 / 50%),
rgb(0 0 255 / 50%)
), url("catfront.png");Source: MDN
There was a problem hiding this comment.
lib/block-supports/background.php
Outdated
| @@ -85,7 +103,9 @@ function gutenberg_render_background_support( $block_content, $block ) { | |||
|
|
|||
| $updated_style .= $styles['css']; | |||
There was a problem hiding this comment.
This is not ideal, and I'm not convinced we should be skipping serialization for for background images as we're doing here:
Color gradients output values using the background shorthand CSS property. Since serialization is not skipped in this case it will exist in the CSS output along with any background-image styles, since the updated CSS is concatenated.
For the record, here's are some stated reasons why they're being skipped:
#53934 (comment) (argument against)
There was a problem hiding this comment.
As it is, where there is an image:
- where a
background: ....;inline CSS rule exists, it needs to be parsed out of the string to make way for the combinedbackground-imagevalue. - where a preset classname exists, e.g.,
has-very-light-gray-to-cyan-bluish-gray-gradient-background, it needs to be removed in favour of an inline CSS rule that uses the CSS var, e.g.,background-image: var(--wp--preset--gradient--very-light-gray-to-cyan-bluish-gray), url(http://localhost:8888/wp-content/uploads/2024/04/white-bellied-swallow.jpg);
|
There are some seriously undesirable gymnastics involved in getting Presets have Variables include, a block has a:
The UX is not great either because it seems random when a background image can be used with a gradient. Hint: it's only visibly beneficial where there are alpha values in a gradient to allow the image to be seen underneath. I'm starting to think a change in the UI might be worth an experiment, whereby we merge the gradient and background image controls and control the styles from one location. Not sure. It's a bit of a mess. |
Fooling around with this today, here's my current thinking about an iterative way to extricate Gutenberg from the background gradient conundrum: The problemGutenberg currently uses the The shorthand CSS property overrides sub properties as detailed in this issue: #60401 Gradient backgrounds should use the Unfortunately, that's now being used by So, Gutenberg needs a way to:
How to tackle the problemBackground images are out of the bag, but site-wide background images and theme.json are not yet in WordPress.
The following headings and text relate to the second option. Why integrate gradient and background image?They should both apply to the same property — What about the current background gradient UI control and the value of
|
|
Nicely laid out @ramonjd! One of the things I really like about the description of the problem is that the proposal plans forward for how to integrate the two when a user does something (adding a gradient or background image). Since gradients are widely used right now, planning this in a way that preserves backwards compat by not trying to migrate any older markup, but just support things as-is, I think is a good path forward. Just to add one more thought, if we were to pursue support for layers in the background (i.e. multiple images or multiple gradients) how might that affect the shape of the |
Excellent question 😄 There's nothing (aside from the safe css filter at the moment) preventing users from using the string value of But parsing and integrating these values with the editor would be tricky so I agree it'd be good to plan it out. One option could be that The two could be exclusive, so either: "background": {
"backgroundImage": {
"source": "theme",
"url": "img/untitled.jpg",
},
}Or "background": {
"backgroundImage": {
"gradient": "linear-gradient(...)",
},
}Then the "backgroundImage" key could support arrays: {
"$schema": "https://schemas.wp.org/trunk/theme.json",
"version": 2,
"styles": {
"background": {
"backgroundImage": [
{
"source": "theme",
"url": "img/untitled.jpg"
},
{
"gradient": "linear-gradient(...)"
}
],
"backgroundPosition": "center center",
"backgroundRepeat": "no-repeat",
"backgroundSize": "cover"
}
}
}This would result in: :where(body) {
background-image: url(http://....untitled.jpg), linear-gradient(...);
background-position: ...
....
}"gradient/url" exclusivity doesn't have to be mandatory, but it could avoid overloading/order problems. This is just one idea I came up with in 10 mins - I'm sure there are better ones! |
Oh, interesting idea! I wonder if we'd want to move the array one level up? It looks like |
Oh yeah, good call. 🎉 Unless I'm missing something, the advantage of supporting two types is that it's possible to implement it later even if the current object structure makes it to core. |
Yes, that's my understanding, too 👍 |
ramonjd
force-pushed
the
try/styles-combine-background-image-values
branch
from
3789393
to
549a495
Compare
|
Flaky tests detected in d4fc571. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9867709576
|
ramonjd
force-pushed
the
try/styles-combine-background-image-values
branch
from
ad7beb0
to
e9b71ef
Compare
| @@ -42,6 +42,139 @@ const migrateAttributes = ( attributes ) => { | |||
| }; | |||
|
|
|||
| const deprecated = [ | |||
| // Version with preset gradient color and background image. | |||
There was a problem hiding this comment.
A note on this.
Merging color gradient and background image is technically a migration, not a deprecation. Custom blocks might need this as well.
Furthermore, if migrating color.gradient from background to background-image, which, in my opinion, should happen some day, we won't want to deprecate n blocks.
Filters or https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/src/api/parser/convert-legacy-block.js might required?
There was a problem hiding this comment.
The main issue is the !important on preset styles.
So, if an existing block has a preset gradient set, and a background image, we'd like to merge the two values into something like background-image: var(--wp--preset--gradient-slug), url(...) but the classname has-slug-gradient-background overwrites all this with background: var(--wp--preset--gradient-slug) !important.
So this deprecation strips the classname and classname attribute.
But then, when resetting the background image, the classname attribute need to be set back.
It makes me think we're going about this the wrong way. Maybe a new property in theme.json or something. I don't know.
There was a problem hiding this comment.
It makes me think we're going about this the wrong way
It's this.
My thinking is fuzzy.
There is no deprecation. Blocks before the background-image migration will continue to work as they did before.
Only newly-added blocks will receive the new feature.
That means blocks that have already added a background image and a preset gradient will continue to work as they did before until they're edited, whereupon they'll be merged into one CSS property.
Deprecations are not required, so I'll remove this change.
ramonjd
force-pushed
the
try/styles-combine-background-image-values
branch
from
e9b71ef
to
58cf7f1
Compare
…und.backgroundImage` into one CSS property `background-image` Merge for block supports Moving logic to style engine Background for gradients will exist if there's no background image Ensuring gradient classnames aren't shown Need to strip classnames Add test deprecation for Group block. Need to do Quote, Pullquote, Post content and Verse PHP linting
ramonjd
force-pushed
the
try/styles-combine-background-image-values
branch
from
d4fc571
to
6bfef3c
Compare
ramonjd
force-pushed
the
try/styles-combine-background-image-values
branch
from
13dc018
to
4ec379a
Compare
Initial test of an experiment to combine
color.gradientandbackground.backgroundImageinto one CSS propertybackground-imageI'm testing merging values from two leafs in the theme.json styles tree:
color.gradientbackground.backgroundImageThe merge also needs to occur where one of those two values is defined by global styles.
It's all a bit messy, which makes me think that a neater approach might be to focus on the
backgroundblock supports and add gradient support there.TODO
What?
Why?
background-imageinstead ofbackground#32787How?
Testing Instructions
Test theme.json
{ "$schema": "../../schemas/json/theme.json", "version": 3, "settings": { "appearanceTools": true, "color": { "defaultGradients": true, "gradients": [ { "slug": "fiery-cavern-of-doom", "gradient": "linear-gradient(180deg, rgba(100,201,231,0.26936712184873945) 0%, rgba(212,183,64,0.5746892507002801) 46%, rgba(255,0,0,0.6335127801120448) 100%)", "name": "Fiery cavern of doom" } ] } }, "styles": { "color": { "gradient": "var(--wp--preset--gradient--fiery-cavern-of-doom)", "text": "#ffffff" }, "background": { "backgroundImage": { "url": "https://images.pexels.com/photos/105819/pexels-photo-105819.jpeg?auto=compress&cs=tinysrgb&w=1260&h=750&dpr=" }, "backgroundSize": "cover", "backgroundAttachment": "fixed" }, "blocks": { "core/quote": { "color": { "gradient": "linear-gradient(180deg, rgba(158,231,100,0.26936712184873945) 0%, rgba(64,212,151,0.5746892507002801) 46%, rgba(0,168,255,0.6335127801120448) 100%)" }, "background": { "backgroundImage": { "url": "https://images.pexels.com/photos/22484288/pexels-photo-22484288/free-photo-of-the-circular-stone-terraces-of-the-inca-ruins.jpeg?auto=compress&cs=tinysrgb&w=1260&h=750&dpr=2" }, "backgroundSize": "cover" }, "dimensions": { "minHeight": "100px" } }, "core/verse": { "color": { "gradient": "var(--wp--preset--gradient--fiery-cavern-of-doom)" }, "background": { "backgroundImage": { "url": "https://images.pexels.com/photos/27269578/pexels-photo-27269578/free-photo-of-marti.jpeg?auto=compress&cs=tinysrgb&w=1260&h=750&dpr=2" }, "backgroundSize": "60px" }, "dimensions": { "minHeight": "100px" } } } } }Test block code.
Testing Instructions for Keyboard
Screenshots or screencast