Fix post editor layout when content block has no attributes. #5221
Conversation
tellthemachines
requested review from
audrasjb,
felixarntz and
mukeshpanchal27
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
Thank you for the PR @tellthemachines. One thing looks off to me here as far as I can tell.
| switch_theme( 'block-theme-post-content-default' ); | ||
|
|
||
| $this->assertSame( $attributes_empty, wp_get_post_content_block_attributes() ); | ||
| } |
There was a problem hiding this comment.
Related to my above feedback, maybe add another test for get_block_editor_settings(), to check that it doesn't include the postContentAttributes field if the post block attributes aren't set.
There was a problem hiding this comment.
The original test already checks the return value for classic themes (when Post Content doesn't exist), do you think it would be worth splitting that check out into its own test?
There was a problem hiding this comment.
@tellthemachines I meant a test for the get_block_editor_settings() change. As far as I can tell, there isn't one at the moment.
There is https://github.com/WordPress/wordpress-develop/blob/trunk/tests/phpunit/tests/blocks/editor.php#L524 which checks the presence of postContentAttributes, so I think we should add another test where we force wp_get_post_content_block_attributes() to return null, and then assert that get_block_editor_settings() does not include the postContentAttributes key.
There was a problem hiding this comment.
Oh got it! Yeah that's a good idea.
remove variable Co-authored-by: Felix Arntz <[email protected]>
mukeshpanchal27
left a comment
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Thanks @tellthemachines for the PR. Left some nit-pick document and unit test changes.
src/wp-includes/block-editor.php
Outdated
| * Retrieves Post Content block attributes from the current post template. | ||
| * | ||
| * @since 6.3.0 | ||
| * @since 6.4.0 return null by default and return empty array when Post Content block exists. but has no attributes. |
There was a problem hiding this comment.
| * @since 6.4.0 return null by default and return empty array when Post Content block exists. but has no attributes. | |
| * @since 6.4.0 return null by default or an empty array when the Post Content block exists. but has no attributes. |
Use assertNull Co-authored-by: Mukesh Panchal <[email protected]>
Update doc. Co-authored-by: Mukesh Panchal <[email protected]>
| switch_theme( 'block-theme-post-content-default' ); | ||
|
|
||
| $this->assertSame( $attributes_empty, wp_get_post_content_block_attributes() ); | ||
| } |
There was a problem hiding this comment.
@tellthemachines I meant a test for the get_block_editor_settings() change. As far as I can tell, there isn't one at the moment.
There is https://github.com/WordPress/wordpress-develop/blob/trunk/tests/phpunit/tests/blocks/editor.php#L524 which checks the presence of postContentAttributes, so I think we should add another test where we force wp_get_post_content_block_attributes() to return null, and then assert that get_block_editor_settings() does not include the postContentAttributes key.
SImplify doc Co-authored-by: Felix Arntz <[email protected]>
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
Thank you for the updates @tellthemachines, LGTM!
|
Thanks for the reviews! Committed in r56629. |
3 participants
Trac ticket: https://core.trac.wordpress.org/ticket/59358
This PR syncs the changes from WordPress/gutenberg#54485 and adds a test for the Post Content block with empty attributes.
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.