Fix layout when post content is root block

[tellthemachines]

What?

In #54371 we reintroduced content size in the post editor when the Post Content block has a default layout type and is nested inside other blocks. However, if Post Content is at root level and has default layout, we should assume that it is meant to stretch full width and let that happen.

The problem is we're using the Post Content attributes to check whether the template being used is a block template; if layout is set to default there are no attributes (because it's using the default layout type) and we don't add anything to the editor settings. This results in the fallback layout (with content width) being used instead of the default.

This PR proposes changing the check that adds the Post Content attributes to the editor settings so that it still outputs an empty array if the block exists but the attributes are empty.

An alternative to this would be using the __unstableIsBlockBasedTheme editor setting to check whether the theme is a block theme, but that wouldn't work for hybrid themes that might have one or two templates with Post Content blocks.

Testing Instructions

1. Create a post template with a Post Content block at root level, and make sure the "Inner blocks use content width" toggle is off;
2. Check that posts using that template have no max content width in the post editor;
3. Check that Post Content blocks with "Inner blocks use content width" toggled on, and with it toggled off but with the block nested inside other blocks, still correctly display a max content width in the post editor.

Testing Instructions for Keyboard

Screenshots or screencast

[github-actions]

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/compat/wordpress-6.3/block-editor-settings.php

[github-actions]

Flaky tests detected in 089d6d7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6193239818
📝 Reported issues:

• [Flaky Test] should partially select with shift + click (@webkit) #53524 in /test/e2e/specs/editor/various/multi-block-selection.spec.js

[ramonjd]

Under which condition is post_content_block['attrs'] an object?

This PR proposes changing the check that adds the Post Content attributes to the editor settings so that it still outputs an empty array if the block exists but the attributes are empty.

If we always want an empty array regardless, would this work instead (removing the if statement)?

$settings['postContentAttributes'] = ! empty( $post_content_block['attrs'] ) ? $post_content_block['attrs'] : array();

I think empty() will return false for objects, even "empty" ones but it wouldn't matter since we'd be returning it or an empty array, which I think is what this PR does anyway.

$empty_object = new stdClass();
var_dump(empty($empty_object)); // false!

[tellthemachines]

Under which condition is post_content_block['attrs'] an object?

If it has attributes in it, though is_array seems to work either way, so perhaps we don't need the second check.

If we always want an empty array regardless

We only want an empty array if the Post Content block exists but doesn't have attributes. For classic themes we still want this to be undefined. The bug I'm trying to fix is with Post Content blocks with default layout, which don't have any explicit attributes; in that case we need to know whether the block exists or not (presumably if it doesn't exist, we're in a classic theme).

[andrewserong]

If switching to an is_array() check, we might need an isset() to go before it, too, to prevent an Undefined index warning in case $post_content_block is empty? Just testing this out locally in PHP:

If it has attributes in it, though is_array seems to work either way, so perhaps we don't need the second check.

If it has attributes in it, then it looks like is_array should still work, because it'll return true if it's an associative array, so I don't think we'd need the is_object check 🤔

[ramonjd]

We only want an empty array if the Post Content block exists but doesn't have attributes. For classic themes we still want this to be undefined.

Ah, got it, thanks!

If it has attributes in it, though is_array seems to work either way, so perhaps we don't need the second check.

I think that'd be safe. In everything under block-supports for example, we're always accessing $block['attrs'] as though it's an array.

[tellthemachines]

If switching to an is_array() check, we might need an isset() to go before it

Actually we could just use isset! That also tells us that the Post Content block exists.

[ramonjd]

Is this meant for 6.3 or 6.4? Assuming 6.3 since it's in the compat/6.3 folder. Just checking!

In 2023 theme, I created a single post template with just a Post Content block in it.

Before - content width

After - no content width (full width)

After (with "Inner blocks use content width" turned on in the post content block)

Inserting the Post Content block (with "Inner blocks use content width" turned off) inside a Group block had the same effect as the last screen shot.

So it LGTM, I just had a question about whether we need to do any type checking at all.

[tellthemachines]

Is this meant for 6.3 or 6.4? Assuming 6.3 since it's in the compat/6.3 folder. Just checking!

Oh, that's a good point. It's meant for 6.4 so I should probably move this into the 6.4 folder.

[tellthemachines]

Actually thinking better of this, we probably don't need it at all because this wasn't an issue before #54371. If I make this fix directly in core, things should work as expected for all versions.

[andrewserong]

Actually thinking better of this, we probably don't need it at all because this wasn't an issue before #54371.

Wouldn't we still need it for folks running WP <= 6.3 and the Gutenberg plugin?

[tellthemachines]

Wouldn't we still need it for folks running WP <= 6.3 and the Gutenberg plugin?

Good point! So I guess this can stay as it is then.

[andrewserong]

Nice fix, and great catch!

In the other PR, I'd only tested when adjusting an existing post content block that already had a layout type set, so toggling the Inner blocks use content width set an explicit type: 'default' for the layout attributes of the block, which meant it displayed properly for me in the editor, so I never noticed the issue!

To reproduce for this one, I created a new post content block in a template without touching the controls, and that revealed the issue 👍

Testing nicely, in a template where Post Content is at the root:

Before	After

LGTM! ✨

[tellthemachines]

Thanks for the reviews folks! I'll put up the core patch soon.