__experimentalBlockPatterns: load them with REST API #39185
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsnajdr
requested review from
TimothyBJacobs,
spacedmonkey,
nerrad,
ajitbohra,
mkaz and
ryanwelcher
as code owners
jsnajdr
force-pushed
the
add/block-patterns-endpoint
branch
from
90ed2a2
to
349af5d
Compare
|
Size Change: +287 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
jsnajdr
force-pushed
the
add/block-patterns-endpoint
branch
from
349af5d
to
9b1ad3d
Compare
youknowriad
added
[Type] Enhancement
I think that's good and is needed because patterns are not only used in the inserter. They are used in every block in block-switcher trying to find pattern transformations, during some blocks insertion, etc.... Because parsing the patterns can take some time, right now we preparse them on load, on browsers idle time and I think this should be preserved. |
| */ | ||
| public function get_items() { | ||
| $data = WP_Block_Patterns_Registry::get_instance()->get_all_registered(); | ||
| return rest_ensure_response( $data ); |
There was a problem hiding this comment.
Please ensure that prepare_item_for_response is implemented here. I would also use prepare_response_for_collection.
This is a good example to follow.
There was a problem hiding this comment.
I implemented prepare_item_for_response, which merely copies the supported fields from the "raw" pattern to the "prepared" one. There's not need to sanitize or escape any fields. The patterns have been either registered by themes and plugins, or downloaded from wordpress.org and already sanitized. There's also an add_additional_fields_to_object call which allows new fields to be registered with register_rest_field( 'block-pattern' ).
I added also a prepare_response_for_collection call, although it's a noop because there are no links in the items.
| * Constructor. | ||
| */ | ||
| public function __construct() { | ||
| $this->namespace = 'wp/v2'; |
There was a problem hiding this comment.
Testing REST API endpoints, use the __experimental namespace.
There was a problem hiding this comment.
Moved to __experimental namespace ✅
spacedmonkey
requested changes
Mar 16, 2022
Still, they should not block the initial load that leads to displaying the post and edit UI. The first time I really need the patterns is when I click on a block and its block controls are displayed, right? Only then there is UI to transform the block to something else, to replace it with another pattern etc. The canonical way to lazy load things is that they are not loaded until their selector is actually called for the first time: useSelect( ( select ) => select( blockEditorStore ).getPatterns() );In our case though, the patterns are loaded much earlier, namely when mounting the editor provider component. I'm not sure how much of a problem this will still be after we fully migrate the load to REST API. The ideal situation though is that they are loaded as late as possible. |
ntsekouras
reviewed
Mar 16, 2022
lib/rest-api.php
Outdated
| $block_patterns = new WP_REST_Block_Patterns_Controller(); | ||
| $block_patterns->register_routes(); | ||
| } | ||
| add_action( 'rest_api_init', 'gutenberg_register_rest_block_patterns' ); |
There was a problem hiding this comment.
We should move this in compat/wordpress-6.0 folder.
| * | ||
| * @return WP_Error|WP_REST_Response Response object on success, or WP_Error object on failure. | ||
| */ | ||
| public function get_items() { |
There was a problem hiding this comment.
Without the $request param this throws a warning.
Suggested change
| public function get_items() { | |
| public function get_items( $request ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable |
jsnajdr
force-pushed
the
add/block-patterns-endpoint
branch
from
9b1ad3d
to
91c80fe
Compare
That's supported without writing any explicit code, because the The endpoint handler could call |
Comment on lines
100
to
108
| $prepared_pattern = array( | ||
| 'name' => $item['name'], | ||
| 'title' => $item['title'], | ||
| 'blockTypes' => $item['blockTypes'], | ||
| 'categories' => $item['categories'], | ||
| 'content' => $item['content'], | ||
| ); | ||
|
|
||
| $prepared_pattern = $this->add_additional_fields_to_object( $prepared_pattern, $request ); |
There was a problem hiding this comment.
Suggested change
| $prepared_pattern = array( | |
| 'name' => $item['name'], | |
| 'title' => $item['title'], | |
| 'blockTypes' => $item['blockTypes'], | |
| 'categories' => $item['categories'], | |
| 'content' => $item['content'], | |
| ); | |
| $prepared_pattern = $this->add_additional_fields_to_object( $prepared_pattern, $request ); | |
| $fields = $this->get_fields_for_response( $request ); | |
| $prepared_pattern = array(); | |
| $keys = array( 'name', 'title', 'blockTypes', 'categories', 'content' ); | |
| foreach( $keys as $key ) { | |
| if ( rest_is_field_included( $key, $fields ) ) { | |
| $prepared_pattern[$key] = $item[$key]; | |
| } | |
| } | |
| $prepared_pattern = $this->add_additional_fields_to_object( $prepared_pattern, $request ); |
Let's try this.
There was a problem hiding this comment.
@jsnajdr This still needs eyes on.
There was a problem hiding this comment.
The _fields list is already being honored, just in a different way, I described it here: #39185 (comment)
Or is it preferred that rest_is_field_included is used explicitly by all new endpoints?
|
|
||
| $prepared_pattern = $this->add_additional_fields_to_object( $prepared_pattern, $request ); | ||
|
|
||
| return new WP_REST_Response( $prepared_pattern ); |
There was a problem hiding this comment.
Suggested change
| return new WP_REST_Response( $prepared_pattern ); | |
| $context = ! empty( $request['context'] ) ? $request['context'] : 'view'; | |
| $data = $this->add_additional_fields_to_object( $prepared_pattern, $request ); | |
| $data = $this->filter_response_by_context( $data, $context ); | |
| $response = rest_ensure_response( $data ); | |
| return $response; |
There was a problem hiding this comment.
Added context support to the endpoint, although in this case it doesn't really make much difference.
| 'title' => array( | ||
| 'description' => __( 'The pattern title, in human readable format.', 'gutenberg' ), | ||
| 'type' => 'string', | ||
| 'readonly' => true, |
There was a problem hiding this comment.
Please ensure that all these properties have 'context' defined.
|
@ntsekouras There are some changes in this PR that are related to #39493 and that you might be the best person to review: the goal is to load the remote patterns only in the
|
spacedmonkey
approved these changes
Mar 28, 2022
jsnajdr
force-pushed
the
add/block-patterns-endpoint
branch
from
8648de4
to
f2edbd5
Compare
What I mean was this. |
That would work if the block pattern was linking to just one block type. Then it's similar to how, e.g., How would the structure look like for array of |
ntsekouras
approved these changes
Mar 28, 2022
There was a problem hiding this comment.
Thanks @jsnajdr! LGTM 👍
I seem to cannot reproduce my previous comment here about the featured category not being fetched and I couldn't see any related changes about it, afterwards, so 🤷 ..
Batching is not possible at all for |
That's right, fixing in #39834. |
36 tasks
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Apr 12, 2022
Backport of WordPress/gutenberg#39185 from the Gutenberg plugin. Namely the part where the `gutenberg_remove_block_patterns_settings` filter function removes the block patterns fields from settings. Props jsnajdr, zieladam. See #55505. Follow-up for [53152]. git-svn-id: https://develop.svn.wordpress.org/trunk@53155 602fd350-edb4-49c9-b593-d223f7449a82
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Apr 12, 2022
Backport of WordPress/gutenberg#39185 from the Gutenberg plugin. Namely the part where the `gutenberg_remove_block_patterns_settings` filter function removes the block patterns fields from settings. Props jsnajdr, zieladam. See #55505. Follow-up for [53152]. Built from https://develop.svn.wordpress.org/trunk@53155 git-svn-id: https://core.svn.wordpress.org/trunk@52744 1a063a9b-81f0-0310-95a4-ce76da25c4cd
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Apr 12, 2022
Backport of WordPress/gutenberg#39185 from the Gutenberg plugin. Namely the part where the `gutenberg_remove_block_patterns_settings` filter function removes the block patterns fields from settings. Props jsnajdr, zieladam. See #55505. Follow-up for [53152]. Built from https://develop.svn.wordpress.org/trunk@53155 git-svn-id: http://core.svn.wordpress.org/trunk@52744 1a063a9b-81f0-0310-95a4-ce76da25c4cd
2 tasks
fullofcaffeine
added a commit
to Automattic/wp-calypso
that referenced
this pull request
May 2, 2022
…r of patterns registered After the changes in WordPress/gutenberg#39185, patterns are not available in the `__experimentalBlockPatterns` symbol anymore.
fullofcaffeine
added a commit
to Automattic/wp-calypso
that referenced
this pull request
May 2, 2022
…r of patterns registered (#63228) After the changes in WordPress/gutenberg#39185, patterns are not available in the `__experimentalBlockPatterns` symbol anymore.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft fix for #39055. We're removing
__experimentalBlockPatternsdata from editor settings, stop sending them in the initial HTML document, and load them from a new REST endpoint instead. The PR has several parts:/wp/v2/block-patternsREST endpoint. Currently, it only supports getting the whole list, not individual items. Gutenberg doesn't need the individual items, but I guess it's a good custom in WP REST APIs to provide access to them, too.getBlockPatterns()selector/resolver to thecore-datastore. It uses the new endpoint to fetch the data.settingsobject is passed toBlockEditorProvider, fill the__experimentalBlockPatternsfield with results of thecore-dataselector if it's not present.__experimentalBlockPatternsfield in the PHP code that generates thewp.editSite.initializeEditorcode.block_editor_settings_all) that removes__experimentalBlockPatternsfield from the settings generated by Core pages likesite-editor.phporedit-form-blocks.php.If this works well, I'll add an endpoint for block pattern categories, too.
There's one thing I don't like about this patch: I used an approach that is already used for
__experimentalReusableBlocks, but it has one downside: instead of loading the block patterns only when they're needed, i.e., when opening the Block Inserter UI, they are loaded when the editor is initialized. So the download is not really delayed, only slightly deferred and parallelized: instead of being a static part of the initial HTML, there's a separate REST request that doesn't block anything.I'll proceed with trying to improve this: the
settingsfield won't be the block pattern list itself, but rather a function to fetch them, used by a resolver. There's an__experimentalFetchLinkSuggestionsfield that works like this.