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.

Sign up for GitHub

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

Fix layout when post content is root block #54485

Merged
merged 3 commits into from
Sep 15, 2023
Merged

Fix layout when post content is root block #54485

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
089d6d7
Fix layout when post content is root block
tellthemachines Sep 15, 2023
3a7dd72
Change check to isset
tellthemachines Sep 15, 2023
5871a64
Update lib/compat/wordpress-6.3/block-editor-settings.php
tellthemachines Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/compat/wordpress-6.3/block-editor-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function gutenberg_get_block_editor_settings_experimental( $settings ) {
$template_blocks = parse_blocks( $current_template[0]->content );
$post_content_block = gutenberg_find_first_block( 'core/post-content', $template_blocks );

if ( ! empty( $post_content_block['attrs'] ) ) {
if ( is_array( $post_content_block['attrs'] ) || is_object( $post_content_block['attrs'] ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

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:

image

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 🤔

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

$settings['postContentAttributes'] = $post_content_block['attrs'];
}
}
Expand Down