-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add a new endpoint that exposes block editor settings through the REST API #29969
Conversation
# Conflicts: # lib/global-styles.php # lib/load.php # lib/rest-api.php
Hey @nosolosw 👋 Since you are currently working on Global styles, I'd appreciate any feedback you might have regarding this PR. Thank you so much! |
Size Change: -159 kB (-11%) 👏 Total Size: 1.31 MB
ℹ️ View Unchanged
|
It would probably be better to build this REST API endpoint in a way that allows using it on the front-end by Gutenberg or any other projects that needs that detail. |
Hey @gziolo thank you for your feedback! I changed the naming a bit so it's just for getting global styles settings in general and removing any mobile-specific mention. |
|
One thing that I'm not sure about is why you think that the settings for Global Styles should be consumed separately from all other settings. In particular, I'm worried that |
👋 I think I miss some context into what mobile wants to do to be more helpful. To offer some feedback, I'm assuming that this is what mobile wants:
Assuming that's the goal, here's some feedback:
|
@@ -101,163 +101,163 @@ public function get_item_schema() { | |||
'__unstableEnableFullSiteEditingBlocks' => array( | |||
'description' => __( 'Enables Full site editing blocks', 'gutenberg' ), | |||
'type' => 'boolean', | |||
'context' => array( 'view' ), | |||
'context' => array( 'post-editor', 'site-editor', 'widget-editor' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@youknowriad, see this context which is meant to allow the mobile app to pick settings for a given block editor type. As discussed in #31027 (comment), we need to figure out what those contexts should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@draganescu, should it be widget-editor
or widgets-editor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@draganescu, should it be
widget-editor
orwidgets-editor
?
@talldan, @noisysocks, @tellthemachines, or @kevin940726 - can you help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
widgets-editor
seems to be preferred everywhere. Does this match another value defined in the codebase? The name of the editor passed to gutenberg_initialize_editor
is currently widgets_editor
with an underscore.
There's also the customizer screen, which is widgets_customizer
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
widgets-editor
it is then. I updated the code in 3897d19. Thank you.
There's also the customizer screen, which is widgets_customizer.
Will it use a different set of settings?
return $this->add_additional_fields_schema( $this->schema ); | ||
} | ||
|
||
$this->schema = array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds great. It looks like there are some items that are off in terms of in which context they're available. Some that I've noticed that should be site-editor
only are: __experimentalGlobalStylesUserEntityId
, __experimentalGlobalStylesBaseStyles
. Other than that, it seems that we can land this PR already? The global styles settings use the old filter block_editor_settings
and we should update it but it doesn't block this PR, as far as I'm aware.
Also: any thoughts on best practices to keep this schema up to date as things evolve would be great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: any thoughts on best practices to keep this schema up to date as things evolve would be great.
If we would start using the same endpoint for the web then it could help to maintain the schema 😄
In related efforts, I plan we switch to the REST API endpoint for block types on the web rather than expose them with JS function call on the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nosolosw !
Some that I've noticed that should be site-editor only are: __experimentalGlobalStylesUserEntityId, __experimentalGlobalStylesBaseStyles
I updated those to be site-editor
only.
The global styles settings use the old filter block_editor_settings and we should update it but it doesn't block this PR, as far as I'm aware.
Awesome 😃! Do you know more or less when that's going to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just merged #31762 which adds more data for mobile. Upon hitting the button, it just occurred to me that something was off. I think it boils down to the contexts we define in this PR: post-editor
, site-editor
, widgets-editor
. Shouldn't we also have a mobile
context?
My understanding is that we use this to filter what kind of data we pass through the request. If the plan is to move the settings retrieval to use the REST request, checks like https://github.com/WordPress/gutenberg/pull/31762/files#diff-0b8e98dead5542e95159bd3c482158111d3f16637e7eef9af13373358fa42dedR91 don't make sense. However, the current contexts are also not enough. I'm thinking we actually need a mobile
context as well. @geriux et all, do you have thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, prepared #31819 to add a mobile
context.
There is one minor question I have #29969 (comment). Once it's confirmed we can land. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied some changes to the REST API schema in 3897d19 and I think everything looks good now. Let's iterate on it as we apply changes to the block editor setting while exposing new features planned for WordPress 5.8.
Awesome! Thank you! |
Does this mean we should revisit the way we initialize the editor? Instead of receiving settings from php, call an API to retrieve them? |
I think we should explore that approach for block editor settings and block types now that we have REST API endpoints that are capable of doing that. |
Created a follow-up for this at #31534 It also looks like we need more structured info through the REST API for mobile (styles). Going to prepare a PR for that soon (hopefully today). |
'allowedBlockTypes' => array( | ||
'description' => __( 'List of allowed block types.', 'gutenberg' ), | ||
'type' => 'boolean', | ||
'context' => array( 'post-editor', 'site-editor', 'widgets-editor' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noted this in other PRs and on Trac, but the lack of post type context constitutes a large functional break for all of the Gutenberg sites I've worked on, and I expect for many others as well. Being able to restrict certain blocks to certain post types is a crucial feature of many advanced block editor implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed with @chrisvanpatten in WordPress/wordpress-develop#1118 (comment) that it should be considered passing additional params in addition to the context to bring feature parity with how the edit post editor works today. It looks like the possibility to change settings per individual post/page is used by the community. It means that we should explore how params like post_id
could be passed and consumed by this endpoint.
#31762 for passing consolidated structured styles for the mobile apps |
Fixes #29063
Description
Adds a new REST API endpoint to expose block editor settings. Useful to get things like global style settings and more.
How has this been tested?
Using a local instance of WordPress:
/__experimental/wp-block-editor/v1/settings
Screenshots
N/A
Types of changes
New rest API endpoint.
Checklist: