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

Synchronize wp_is_block_theme and block-templates block support with Core #37218

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

youknowriad
Copy link
Contributor

In this PR I'm continuing the audit of the Gutenberg plugin php code and aligning behavior with Core.

See #37141 for the reasoning.

@youknowriad youknowriad added [Type] Code Quality Issues or PRs that relate to code quality Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Dec 8, 2021
@youknowriad youknowriad self-assigned this Dec 8, 2021
* Enable block templates (editor mode) for themes with theme.json.
*/
function gutenberg_enable_block_templates() {
if ( wp_is_block_theme() || WP_Theme_JSON_Resolver_Gutenberg::theme_has_support() ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this function I added wp_is_block_theme to auto-enable block-templates for block themes. It was already the case in the Gutenberg plugin because before this PR, the plugin used gutenberg_supports_block_templates to check support but for core, if a block theme didn't have a "theme.json" file, block templates support would have been disabled. So this change here need to be backported into Core.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you already made this change in Core so I'll do nothing 😀

WordPress/wordpress-develop@f7d2a2e

@@ -208,7 +208,7 @@ function gutenberg_get_default_block_editor_settings() {
}

$editor_settings = array(
'__unstableEnableFullSiteEditingBlocks' => gutenberg_supports_block_templates(),
'__unstableEnableFullSiteEditingBlocks' => current_theme_supports( 'block-templates' ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that this __unstableEnableFullSiteEditingBlocks flag is not set on Core at all, I'm not entirely sure what's the impact here but potentially we have a missing backport as well cc @noisysocks @gziolo

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes there was a ticket for that.

https://core.trac.wordpress.org/ticket/54583

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's actually another flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I just saw that all the usage of __unstableEnableFullSiteEditingBlocks is actually inside process.env.GUTENBERG_PHASE === 2 meaning it's not necessary at all to backport this to Core.

lib/compat/wordpress-5.9/theme.php Outdated Show resolved Hide resolved
Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
lib/compat/wordpress-5.9/move-theme-editor-menu-item.php Outdated Show resolved Hide resolved
lib/compat/wordpress-5.9/theme.php Outdated Show resolved Hide resolved
lib/full-site-editing/full-site-editing.php Outdated Show resolved Hide resolved
lib/navigation.php Outdated Show resolved Hide resolved
Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@youknowriad youknowriad merged commit 7762302 into trunk Dec 8, 2021
@youknowriad youknowriad deleted the sync/fse-utils-core branch December 8, 2021 11:57
@youknowriad
Copy link
Contributor Author

Just noting that there are two backports to be done as a result of this PR so we don't forget :)

@github-actions github-actions bot added this to the Gutenberg 12.2 milestone Dec 8, 2021
@scruffian
Copy link
Contributor

I think this introduced a regression. I can no longer switch to block themes since this PR merged.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 8, 2021

@scruffian, I also noticed that. It should be fixed via #37226.

@youknowriad youknowriad added Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Dec 9, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 13, 2021
noisysocks pushed a commit that referenced this pull request Dec 13, 2021
…Core (#37218)

Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants