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

Backport background size and repeat block support features from Gutenberg #5828

Conversation

andrewserong
Copy link
Contributor

@andrewserong andrewserong commented Jan 2, 2024

Add support for background size and background repeat to the background image block support, as used by the Group block.

This PR backports the PHP parts of the following Gutenberg PRs: WordPress/gutenberg#57005, WordPress/gutenberg#57474, and WordPress/gutenberg#57495

This PR only backports the PHP parts of the feature, however it can safely land prior to the JS packages updates for WP 6.5 as users will not be able to manually use these features in the editor until the JS packages are updated.

Testing instructions

In a post or page of a site running TT4 theme, add the following markup in the code editor view. It contains a Group block with a background image set to a size of contain and a repeat of no-repeat:

<!-- wp:group {"style":{"background":{"backgroundImage":{"url":"https://i0.wp.com/wordpress.org/files/2022/10/community-photo-2-q50-unscaled.webp?w=2642","source":"file"},"backgroundSize":"contain","backgroundRepeat":"no-repeat"}},"backgroundColor":"cyan-bluish-gray","layout":{"type":"constrained"}} -->
<div class="wp-block-group has-cyan-bluish-gray-background-color has-background"><!-- wp:heading -->
<h2 class="wp-block-heading">A group block with a background image</h2>
<!-- /wp:heading --></div>
<!-- /wp:group -->

The view in the editor will not reflect the background size and repeat rules yet, however with this PR, the site frontend's view should reflect that the image is contained within the area of the Group, is centered by default, and is set to not repeat:

image

Trac ticket: https://core.trac.wordpress.org/ticket/60175


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Copy link

github-actions bot commented Jan 2, 2024

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@@ -40,6 +40,8 @@ function wp_register_background_support( $block_type ) {
* it is also applied to non-server-rendered blocks.
*
* @since 6.4.0
* @since 6.5.0 Added support for `backgroundSize`, `backgroundPosition`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backgroundSize was added in 6.4?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Updated in 6a82e04. The output of backgroundSize was added in 6.4 but the UI controls weren't. Since this function already handled output for backgroundSize, I've updated the comment here to just reflect position and repeat output.

@andrewserong andrewserong force-pushed the add/background-size-and-repeat-block-support-features branch from 6a82e04 to fc04c45 Compare January 7, 2024 23:38
@andrewserong
Copy link
Contributor Author

Small update to also include the changes from WordPress/gutenberg#57495 in this backport.

@andrewserong andrewserong force-pushed the add/background-size-and-repeat-block-support-features branch from 7696907 to 185ee76 Compare January 9, 2024 00:31
Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Just one question below, otherwise LGTM.
Testing works as expected:

Trunk:
Screenshot 2024-01-09 at 1 35 33 pm
This branch:
Screenshot 2024-01-09 at 1 35 43 pm

@@ -67,6 +68,12 @@ function wp_render_background_support( $block_content, $block ) {
$background_size = isset( $block_attributes['style']['background']['backgroundSize'] )
? $block_attributes['style']['background']['backgroundSize']
: 'cover';
$background_position = isset( $block_attributes['style']['background']['backgroundPosition'] )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we check if a background image url exists as part of the condition that checks for block support above? That way we could return early if there is no image. Or are there reasons to still go through this logic in the absence of a url?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do an early return. It was mostly laid out like it currently is to support subsequent updates where we might have a range of different values for the background image source. I.e.

  • 'featured_image' === $background_image_source
  • 'id' === $background_image_source (for referencing a media library item directly), etc

So we could do an early return just before we define $background_size, something like:

if ( ! $background_image_source && ! $background_image_url ) {
  return $block_content;
}

Would that be worth doing, do you think?

Copy link
Contributor

@tellthemachines tellthemachines Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't those sources still require defining a URL though? We always need the URL in order to output the styles. But yeah I guess in any case, in subsequent updates we can add to the condition if that changes.

Edit: to be clear, I do think it's worth returning as early as possible 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okie-doke! I'll update with an early return 👍

For the featured_image case we won't have a url in the block attributes, as that would get generated at render time. But that could happen anywhere we like in the function, so we could always put that logic before this $background_position line and do an early return if we don't have a featured image, either. Let's take a look at that part when we go to implement the feature 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an early return in c2982a3 — let me know if you'd like me to tweak it!

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the iteration! This is looking good now 👍

@tellthemachines
Copy link
Contributor

Committed in r57254.

@andrewserong
Copy link
Contributor Author

Thanks for reviewing and committing! 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants