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

Layout: Fix issue where layout classnames are injected for blocks without layout support #56187

Conversation

andrewserong
Copy link
Contributor

What?

This resolves an issue found while testing #56130 (review) — if a child of a Row block is explicitly set to Fit then layout classnames are erroneously injected for blocks that do not support layout (e.g. a paragraph block set to Fit as a child of the Row block).

Why?

Layout classnames should not be output for blocks without layout support. They were being injected because we didn't have an early return condition for if a block has selfStretch set, doesn't have layout support, but does not generate any styles.

How?

Following the condition for outputting the outer wrapper classname for selfStretch rules, perform an early return if the block does not support layout.

Testing Instructions

  1. Add a Row block with a few child Paragraph blocks and set one of the Paragraphs to be explicitly Fit (i.e. set it to Fill, then set it back to Fit)
  2. Save the post and view the markup on the site frontend. Notice that on trunk extra layout classnames are injected into the paragraph block.
  3. With this PR applied, there should be no unexpected classnames injected to the paragraph block.
  4. Switch the paragraph block to use an explicit fixed width. After saving and reloading, it should get its expected wp-container-content-x classname and associated styles.

Test markup:

<!-- wp:group {"layout":{"type":"flex","flexWrap":"nowrap"}} -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>Paragraph 1</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph {"style":{"layout":{"selfStretch":"fit","flexSize":null}}} -->
<p>Paragraph 2</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Paragraph 3</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

Screenshots or screencast

Before

image

After

image

@andrewserong andrewserong added [Type] Bug An existing feature does not function as intended [Feature] Layout Layout block support, its UI controls, and style output. labels Nov 16, 2023
@andrewserong andrewserong self-assigned this Nov 16, 2023
Copy link

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/layout.php

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Before

<div class="wp-block-group is-nowrap is-layout-flex wp-container-core-group-layout-4 wp-block-group-is-layout-flex">
    <p>Paragraph 1</p>
    <p class="is-layout-flow wp-block-paragraph-is-layout-flow">Paragraph 2</p>
    <p>Paragraph 3</p>
</div>

After

<div class="wp-block-group is-nowrap is-layout-flex wp-container-core-group-layout-4 wp-block-group-is-layout-flex">
    <p>Paragraph 1</p>
    <p>Paragraph 2</p>
    <p>Paragraph 3</p>
</div>

🍺

Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

Can confirm exactly the same as Ramon above 👍

@andrewserong
Copy link
Contributor Author

Thanks for the reviews!

@andrewserong andrewserong merged commit 802c7a5 into trunk Nov 16, 2023
56 checks passed
@andrewserong andrewserong deleted the fix/layout-classnames-being-injected-for-blocks-without-layout branch November 16, 2023 23:02
@github-actions github-actions bot added this to the Gutenberg 17.2 milestone Nov 16, 2023
@tellthemachines
Copy link
Contributor

Thanks for fixing this!

@andrewserong
Copy link
Contributor Author

Backport open in WordPress/wordpress-develop#5900

@getdave getdave added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Needs PHP backport Needs PHP backport to Core labels Jan 23, 2024
@getdave
Copy link
Contributor

getdave commented Jan 23, 2024

✅ I updated this PR with the Backported to Core label to indicate that the backport has successfully merged into WP Core. I also updated the PHP Sync Tracking Issue for WP 6.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants