Global styles: block background UI controls #60100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 tasks
|
Size Change: +280 B (+0.02%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
ramonjd
force-pushed
the
add/background-global-styles-panel-for-blocks
branch
from
bdc2180
to
7788359
Compare
ramonjd
commented
Mar 26, 2024
ramonjd
force-pushed
the
add/background-global-styles-panel
branch
from
b09f29e
to
653d32a
Compare
ramonjd
force-pushed
the
add/background-global-styles-panel-for-blocks
branch
from
fad270f
to
a7d3a88
Compare
ramonjd
commented
Mar 27, 2024
ramonjd
commented
Mar 27, 2024
| "additionalProperties": false | ||
| }, | ||
| { | ||
| "$ref": "#/definitions/refComplete" |
There was a problem hiding this comment.
Most of this is copy/paste, double check validity
ramonjd
added
[Type] Enhancement
ramonjd
commented
Apr 3, 2024
| @@ -232,6 +241,19 @@ function ScreenBlock( { name, variation } ) { | |||
| setStyle( { ...newStyle, border: { ...updatedBorder, radius } } ); | |||
| }; | |||
|
|
|||
| const onChangeBackground = ( newStyle ) => { | |||
There was a problem hiding this comment.
TODO: This is not required. We need to set the default-background size/position in the incoming style object if it's not already in getStylesDeclarations in packages/block-editor/src/components/global-styles/use-global-styles-output.js
blockStyles = {
...blockStyles,
background: {
...blockStyles.background,
...setBackgroundStyleDefaults( blockStyles.background ),
}
};There was a problem hiding this comment.
ramonjd
force-pushed
the
add/background-global-styles-panel-for-blocks
branch
from
3249329
to
0582518
Compare
ramonjd
commented
Apr 11, 2024
packages/block-editor/src/components/global-styles/use-global-styles-output.js
Show resolved
Hide resolved
5 tasks
ramonjd
force-pushed
the
add/background-global-styles-panel-for-blocks
branch
from
c8edc3b
to
05f0dd9
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/class-wp-theme-json-gutenberg.php ❔ lib/class-wp-theme-json-resolver-gutenberg.php ❔ lib/global-styles-and-settings.php ❔ phpunit/class-wp-theme-json-resolver-test.php ❔ phpunit/class-wp-theme-json-test.php |
|
Flaky tests detected in dc18794. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9511927706
|
ramonjd
commented
Jun 5, 2024
| @@ -820,6 +821,33 @@ public static function get_resolved_theme_uris( $theme_json ) { | |||
| $resolved_theme_uris[] = $resolved_theme_uri; | |||
| } | |||
|
|
|||
| // Block styles. | |||
| if ( ! empty( $theme_json_data['styles']['blocks'] ) ) { | |||
There was a problem hiding this comment.
Probably should abstract this logic. There'll be elements and other things later I imagine.
Also add unit tests to cover this.
There was a problem hiding this comment.
What did you have in mind for the abstraction? A separate method, or function call to another file? Just curious how this part of the feature might evolve 🙂
There was a problem hiding this comment.
Maybe once (if?) we support background images for elements, the whole foreach could be a function that takes an array of either blocks or elements so we can run it on both? I don't think it's worth abstracting until that happens though.
There was a problem hiding this comment.
I don't think it's worth abstracting until that happens though.
Good point. Let's cross that bridge when we come to it.
What did you have in mind for the abstraction? A separate method, or function call to another file? Just curious how this part of the feature might evolve
I haven't really thought about it, but the obvious target would be around building the resolved array items. So the if block starting from wp_check_filetype().
The future will involve target specific styles — top-level, blocks, elements and, eventually, variations, the latter two requiring possibly further nested loops.
Maybe once elements comes along it'll be clearer how the abstraction should look.
…so a background block control can determine whether it has an inherited value. If there is an inherited value, the block control will provide an option to "remove" the theme style. This commit also sends resolved theme asset URI via the _links property so that these images can be used in block control previews. Added resolver tests for blocks Use `get_styles_for_block` in unit tests when testing block nodes Use `get_styles_for_block` in unit tests when testing block nodes Add since annotation backport changelog
…styles complete ref
…relative paths Also, output any background styles, even if there's no background image
ramonjd
force-pushed
the
add/background-global-styles-panel-for-blocks
branch
from
bc88b69
to
d040811
Compare
|
Thanks for the detailed review and feedback @andrewserong Fortunately those two bugs you caught were human (my) oversight.
I wasn't passing the resolved theme paths 🤦🏻 Fixed!
YES! I have fixed this. There was a check in background.php to only output these values if there was a background image, but I think now that we have support for block styles in theme.json (in this PR) we'll need to allow all properties through. I've fixed that. |
andrewserong
approved these changes
Jul 5, 2024
There was a problem hiding this comment.
Thanks for the follow-up, that's all testing well for me now!
| Block level in post editor when global style is set | Block level sets size, image is from global style (frontend) |
|---|---|
 |
 |
The code change looks good to me, too. I see there was a question (#60100 (comment)) about the use of the globalStylesLinksDataKey. Since that's effectively private, I reckon this change is good to try for now, and we could always look at moving things around in follow-ups if need be?
LGTM! 🚀
|
I've a few things today so just looking at your helpful screenshots (thanks Andrew); can the position thumbnail be the full width of the flyout? |
It would look a lot prettier. There was a bit of to and fro with this one over at #60151 (comment) I had to restrict the size of the image to ensure the popover height didn't blow out. The images can be very very short, or very very tall - it's not possible to know in advance, and allowing them to fill the width ends up with things like this: 
The only alternative I can think of right now is to do some JS math to get the scaled ratio of w/h and then enforce a limit if the ratio is over the threshold of what looks weird, technically speaking 😄 |
|
At worst, we can make the parent container full-width, and throw a gray background color inside, so it's an image inside a frame. That would also help with the super tall thin ones, then the image can be scaled down to be small inside such a gray container. Something along the lines of: That would fit in a full-width gray box with a max-height of, say, 180px. I'm sure there's all sorts of complexity in getting that to work, though! |
|
Thanks for the help and feedback everyone!
Great idea @jasmussen ! I'll have a play around in a follow up. 🙇🏻 Follow ups:
|
ramonjd
added a commit
to ramonjd/wordpress-develop
that referenced
this pull request
Jul 8, 2024
Tests to come
This was referenced Jul 8, 2024
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
* Adding background panel for blocks. * Default controls using block supports * Updating tests after rebase Removed unrequired change function * Set defaults for the global styles block group background image * Enabling theme file resolution for blocks * LINTO * Use global settings to check if background image is supported. * This commit checks for theme block styles in the block supports hook so a background block control can determine whether it has an inherited value. If there is an inherited value, the block control will provide an option to "remove" the theme style. This commit also sends resolved theme asset URI via the _links property so that these images can be used in block control previews. Added resolver tests for blocks Use `get_styles_for_block` in unit tests when testing block nodes Use `get_styles_for_block` in unit tests when testing block nodes Add since annotation backport changelog * update assertion comments * Now that background styles can be used in block styles, move them to styles complete ref * Pass resolved URIs to display inherited background image styles with relative paths Also, output any background styles, even if there's no background image 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> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
This was referenced Jul 22, 2024
This was referenced Jul 23, 2024
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Jul 24, 2024
Allows defining background images for blocks in theme.json. Syncs PHP changes from WordPress/gutenberg#60100. Props ramonopoly, aaronrobertshaw. Fixes #61588. git-svn-id: https://develop.svn.wordpress.org/trunk@58797 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Jul 24, 2024
Allows defining background images for blocks in theme.json. Syncs PHP changes from WordPress/gutenberg#60100. Props ramonopoly, aaronrobertshaw. Fixes #61588. Built from https://develop.svn.wordpress.org/trunk@58797 git-svn-id: http://core.svn.wordpress.org/trunk@58193 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Jul 24, 2024
Allows defining background images for blocks in theme.json. Syncs PHP changes from WordPress/gutenberg#60100. Props ramonopoly, aaronrobertshaw. Fixes #61588. Built from https://develop.svn.wordpress.org/trunk@58797 git-svn-id: https://core.svn.wordpress.org/trunk@58193 1a063a9b-81f0-0310-95a4-ce76da25c4cd
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Enables background images for supported blocks in global styles.
Kapture.2024-07-03.at.16.04.04.mp4
Blocks include:
Part of:
Core backport PR:
Why?
To enable background images across all supported blocks at once! MWHAHAHAHAHA!
How?
Enabling background style properties as valid styles in
VALID_STYLES, and not just valid'top'styles.Testing Instructions
To be updated...
In the Site Editor, open a template that contains a few
GroupBlocks.Then head over to Styles > Blocks >
Groupand set a background image for all Group blocks.Check that all Group blocks now have that background image as default.
Ensure that the default background image is overridable at the block level. That is, select a Group block and set a different background image for that block.
Now define a background image at the block theme in theme.json.
Here's an example using some local files:
{ "$schema": "../../schemas/json/theme.json", "version": 3, "settings": { "appearanceTools": true }, "styles": { "blocks": { "core/verse": { "background": { "backgroundImage": { "url": "file:./img/budgie4.jpg" } } }, "core/quote": { "background": { "backgroundImage": { "url": "file:./img/budgie2.jpgg" } } }, "core/pullquote": { "background": { "backgroundImage": { "url": "file:./img/budgie2.jpg" } } }, "core/post-content": { "background": { "backgroundImage": { "url": "file:./img/budgie1.jpg" } } } } } }Check that this is displayed correctly in the frontend and editor.