Add WP_Block_Editor_Context::$name

[noisysocks]

Trac ticket: https://core.trac.wordpress.org/ticket/55301
Original Gutenberg issue: WordPress/gutenberg#28517

What this is

Adds new WP_Block_Editor_Context::$type and WP_Block_Editor_Context::$is_customizer fields. This allows plugin developers to tell which block editor is being loaded when using filters such as allowed_block_types_all and block_editor_rest_api_preload_paths.

How to test

Here's some code you can put into e.g. wp-content/mu-plugins/allowed-blocks.php:

add_filter(
	'allowed_block_types_all',
	function( $allowed_block_types, $block_editor_context ) {
		if ( 'core/edit-site' === $block_editor_context->name ) {
			return array( 'core/paragraph' );
		}
		if ( 'core/edit-widgets' === $block_editor_context->name ) {
			return array( 'core/heading' );
		}
		if ( 'core/customize-widgets' === $block_editor_context->name ) {
			return array( 'core/image' );
		}
		if ( 'core/edit-post' === $block_editor_context->name ) {
			return array( 'core/separator' );
		}
		return $allowed_block_types;
	},
	10,
	2
);

---

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.

[talldan]

The part I'm most unsure about here is is_customizer, which seems very specific to the widgets use case. WP_Block_Editor_Context maybe shouldn't be that specific.

If the widget editor type values are changed to customize-widgets or edit-widgets, at least someone implementing a filter can do something like this:

add_filter(
	'allowed_block_types_all',
	function( $allowed_block_types, $block_editor_context ) {
		if (
		    'customize-widgets' === $block_editor_context->type ||
		    'edit-widgets' === $block_editor_context->type
		) {
			return array( 'core/paragraph', 'core/heading' );
		}
		return $allowed_block_types;
	},
	10,
	2
);

However, I'm not sure if type is the correct semantic if that naming is used.

edit: another option for consistency is to use the names that we're using in JavaScript ('core/edit-post', 'core/edit-widgets', 'core/customize-widgets', 'core/edit-site'). The interface and preferences APIs are calling this 'scope'. Not sure if that's a good name.

[gziolo]

I'm glad we finally have enough data to improve the implementation of the WP_Block_Editor_Context class. That was exactly the goal here, to be able to customize settings based on conditions like the editor screen type.

I will leave the details for the new class properties to @noisysocks and @talldan as they know better the requirements here. I just wanted to emphasize that the overall proposal is sound 👍🏻

[draganescu]

I agree with @talldan we should only rely on $type. The $is_customizer flag seems redundant.

[noisysocks]

If the widget editor type values are changed to customize-widgets or edit-widgets, at least someone implementing a filter can do something like this:

I did consider this but I don't like it 😅

API design should nudge developers towards doing the right thing. I want developers to configure the block editor exactly the same way for the widgets editor in the widgets screen and the widgets editor in the customizer. They are the same editor and so should provide the same user experience. By having both type and is_customizer, it makes it slightly more difficult for developers to do the wrong thing and configure both editors differently. By having only type, it makes it slightly more difficult for developers to do the right thing and configure both editors the same.

The part I'm most unsure about here is is_customizer, which seems very specific to the widgets use case. WP_Block_Editor_Context maybe shouldn't be that specific.

I'd argue that $is_customizer isn't that specific to the widget use case as I could imagine that a block editor may one day be rendered in the Customizer for some other purpose e.g. to edit a post or page.

Moreover, WP_Block_Editor_Context already has $post which is very specific to type = 'post'.

Ideally I think WP_Block_Editor_Context should be a base class with subclasses to denote the editor type.

abstract class WP_Block_Editor_Context {
}
class WP_Post_Block_Editor_Context extends WP_Block_Editor_Context {
	public $post;
}
class WP_Widgets_Block_Editor_Context extends WP_Block_Editor_Context {
	public $is_customizer;
}
class WP_Site_Block_Editor_Context extends WP_Block_Editor_Context {
}

We can't do this though as it requires renaming WP_Block_Editor_Context to WP_Post_Block_Editor_Context which would break backwards compatibility.

So instead I'm leaning into WP_Block_Editor_Context being a wrapper around an array that really only exists for type hinting purposes.

[talldan]

I'd argue that $is_customizer isn't that specific to the widget use case as I could imagine that a block editor may one day be rendered in the Customizer for some other purpose e.g. to edit a post or page.

My feeling on this isn't particularly strong, I just think this is based on a use case of one, and there are no concrete plans to add more customizer editors. Usually we work on the basis of being terrified by future maintainability, so it seems a little outside the norm to be this bold.

Anyway, our editors are a bit weird, and I don't think names or types really mean that much. We have one 'post' editor that is used to also edit pages and any custom post type. We have a site editor that can literally edit anything and allowed blocks won't mean very much there. Then we have two widget editors that can only edit the same individual type of thing. If anything it might be best to work on the basis the data type being edited (which is I suppose what $post tried to do).

There's no real consistency, so we're probably only able to implement something with trade-offs. I think that's why a prefer to use names that are already in regular usage (core/edit-post, core/edit-site etc.). Plugin devs might already be familiar with these terms.

[noisysocks]

I do like that core/edit-post, core/edit-site, etc. leaves room for plugin authors to use block context by setting the name to woocommerce/product or whatever.

[noisysocks]

Let's make @draganescu decide 😛

[gziolo]

Waiting for Andrei to decide... 🍿 🦗

Ideally I think WP_Block_Editor_Context should be a base class with subclasses to denote the editor type.

Yes, that would work, too. It would be a bit harder to document it though because you would have 5 class names with different class properties. In the future, we might also have a site editor context that allows editing post content that will require $post so it's hard to predict the implications.

[noisysocks]

OK, I updated this to use $name. Semantically it's all the same. I like that this means there's consistency between the strings you see in WP_Block_Editor_Context::$name and the strings you see in wp.data.

[noisysocks]

I opened WordPress/gutenberg#39299 in the Gutenberg repo to compliment this PR. It updates some usages of WP_Block_Editor_Context in the plugin to pass an appropriate $name.

[talldan]

This and the Gutenberg PR (WordPress/gutenberg#39299) look good to me 👍

[noisysocks]

Thanks all. Committed in https://core.trac.wordpress.org/changeset/52942.

[joyously]

Is this available on the 'enqueue_block_editor_assets' action?

[talldan]

@joyously It doesn't look like it as that action doesn't seem to receive the block editor context:

wordpress-develop/src/wp-admin/edit-form-blocks.php

Line 265 in 0fb4f90

do_action( 'enqueue_block_editor_assets' );

It may be possible to add it—I'd recommend making a core trac ticket.