Send registered patterns in HTML and combine them with REST ones #40818
Conversation
| '__experimentalBlockPatterns' => WP_Block_Patterns_Registry::get_instance()->get_all_registered(), | ||
| '__experimentalBlockPatternCategories' => WP_Block_Pattern_Categories_Registry::get_instance()->get_all_registered(), |
There was a problem hiding this comment.
Can't you get data via the REST API? Why load this in via settings?
There was a problem hiding this comment.
The REST API doesn't know all registered patterns. If they are registered in the current_screen or the admin_init hook, these hooks are not run while serving a REST endpoint. See the PR description and the #40736 that this PR attempts to fix.
There was a problem hiding this comment.
If you register your pattern in correctly or weirdly, that is on you as a developer. I would strongly recommend asyncly loading pattern via the REST API. This could be a lot of data on the page load, even if it is not used.
There was a problem hiding this comment.
@spacedmonkey See related conversation about this here: #40736
It was suggested to throw a _doing_it_wrong if not using init, but ultimately decided to maintain backwards compatibility.
|
Size Change: +221 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
adamziel
added
the
Backport to WP 6.7 Beta/RC
There was a problem hiding this comment.
✅ It works
✅ It solves a regression as discussed in #40736
✅ The code makes sense as it merges the patterns from settings with those from the REST API
Let's 🚢 !
@jsnajdr, is there any particular order in which this and WordPress/wordpress-develop#2672 must be merged? I assume we need to backport this to RC3, too.
The Gutenberg patch works even without the Core patch, but vice versa, when Core is patched and Gutenberg unpatched, block pattern loading will break. So, the correct order is Gutenberg then Core. There will be problem when you have patched Core and older Gutenberg plugin, like the version 13.2 released last week. Then you're in the broken state 🙁 |
|
@jsnajdr would it be possible to only pass the |
|
The code in Core that generates the editor HTML could apply a new filter: if ( apply_filters( 'should_add_block_patterns_to_editor_settings', false ) ) {
// Add patterns to settings.
}and then Gutenberg plugin (and also Gutenberg shipped in Core) enables the loading: add_filter( 'should_add_block_patterns_to_editor_settings', '__return_true' );The result is:
Sounds good? |
|
@jsnajdr as far as I understand:
Overall, it does sound really good 🎉 |
jsnajdr
force-pushed
the
update/patterns-from-settings
branch
from
bfd508a
to
a416b84
Compare
Pushed a commit to this PR that implements the filter. When backporting to Core, the
|
jsnajdr
force-pushed
the
update/patterns-from-settings
branch
from
a416b84
to
0fd71b2
Compare
jsnajdr
force-pushed
the
update/patterns-from-settings
branch
from
0fd71b2
to
d64536d
Compare
|
@jsnajdr would you please spin a backport PR for this? Just to recap my understanding, I believe we expect to see the patterns being loaded using the following data sources:
I wonder what would be the best way to achieve that. One thing that comes to mind is ✅ It would get the job done Alternatively, it could be placed in some filter that Gutenberg unhooks or a class that Gutenberg overrides. It would have to be something that already existed in the plugin for a while and was designed for a scenario such as ours. I don't see any obvious candidates for that so maybe |
There was a problem hiding this comment.
Alternatively, it could be placed in some filter that Gutenberg unhooks or a class that Gutenberg overrides. It would have to be something that already existed in the plugin for a while and was designed for a scenario such as ours. I don't see any obvious candidates for that so maybe
is_plugin_activeisn't that bad after all?
is_plugin_active() doesn't return the correct result for sites using the plugin as an mu-plugin.
In the past a check has been used during the WP upgrade procedure
- WP 5.0: Disabled the plugin upon the block editor becoming part of core
- WP 5.9: Disabled plugin versions prior to 11.9 as it was incompatible and caused a fatal error.
I think similar has been in some releases in between but I don't recall which.
I'd prefer a filter rather than adding plugin checks to other source code, it's really snowflakey and sets a bad precedent.
Would reversing the filter's default value help for gutenberg?
| * | ||
| * @param bool $should_add_to_settings | ||
| */ | ||
| if ( apply_filters( 'should_add_block_patterns_to_editor_settings', false ) ) { |
There was a problem hiding this comment.
| if ( apply_filters( 'should_add_block_patterns_to_editor_settings', false ) ) { | |
| if ( apply_filters( 'gutenberg_add_block_patterns_to_editor_settings', false ) ) { |
🔢 Would need to be renamed throughout
It's not possible because the The default value needs to be "don't send me patterns in settings" |
|
A solution even better than the new filter would be to rename the settings fields to:
This avoids the unfortunate condition that shipped in Gutenberg 13.1 that patterns are loaded from REST only when they are not in settings, like: const blockPatterns = settings.__experimentalBlockPatterns ?? select( coreStore ).getBlockPatterns();which disables loading from REST when the setting is present. Then WordPress 6.0 would behave like:
|
Implemented this as an additional commit to both the Core and Gutenberg PRs and verified that it really works 👍 |
@adamziel Not sure how to do this because i) no PHP from this PR needs to be backported, all the PHP changes that are needed are in WordPress/wordpress-develop#2672 ii) we need to backport the JS changes in |
|
@jsnajdr to recap:
Since the Gutenberg version bundled in the WordPress 6.0 is 13.0, I think that's a good trade-off. We could even go one step further and disable plugin versions <= 12.9 on upgrade as @peterwilsoncc have mentioned above. In any case, let's land this 🎉 |
Just one caveat in this setup: WordPress 5.9 with latest Gutenberg will stop showing patterns registered in Can be fixed by looking at both fields: patterns = union( settings.__experimentalBlockPatterns, settings.__experimentalAdditionalBlockPatterns, restPatterns );Going to push a fix. |
) * Send registered patterns in HTML and combine them with REST ones * Fix formatting * Use the outside_init_only param when getting patterns in edit-site page * Add patterns to settings only if enabled by filter * Remove filter and rename settings fields instead * Support all settings forms in both WP 5.9 and 6.0
|
I just cherry-picked this to wp/6.0 branch to be included in the next release: 6c96f52 |
adamziel
removed
the
Backport to WP 6.7 Beta/RC
|
Thank you so much for all your great work here @jsnajdr! 🎉 |
Reverts changes from [53155] to ensure backward compatibility. Companion to Gutenberg changes WordPress/gutenberg#40818. That makes sure that patterns registered with `admin_init` or `current_screen` hooks are not lost. Props jsnajdr, zieladam, peterwilsoncc, johnstonphilip. See #55567. git-svn-id: https://develop.svn.wordpress.org/trunk@53404 602fd350-edb4-49c9-b593-d223f7449a82
Reverts changes from [53155] to ensure backward compatibility. Companion to Gutenberg changes WordPress/gutenberg#40818. That makes sure that patterns registered with `admin_init` or `current_screen` hooks are not lost. Props jsnajdr, zieladam, peterwilsoncc, johnstonphilip. See #55567. Built from https://develop.svn.wordpress.org/trunk@53404 git-svn-id: http://core.svn.wordpress.org/trunk@52993 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Reverts changes from [53155] to ensure backward compatibility. Companion to Gutenberg changes WordPress/gutenberg#40818. That makes sure that patterns registered with `admin_init` or `current_screen` hooks are not lost. Props jsnajdr, zieladam, peterwilsoncc, johnstonphilip. See #55567. Built from https://develop.svn.wordpress.org/trunk@53404 git-svn-id: https://core.svn.wordpress.org/trunk@52993 1a063a9b-81f0-0310-95a4-ce76da25c4cd
mburridge
added
the
Needs Dev Note
|
Not sure if this needs a dev note for the 6.1 release, but adding the label just in case. |
Reverts changes from [53155] to ensure backward compatibility. Companion to Gutenberg changes WordPress/gutenberg#40818. That makes sure that patterns registered with `admin_init` or `current_screen` hooks are not lost. Props jsnajdr, zieladam, peterwilsoncc, johnstonphilip. See #55567. Built from https://develop.svn.wordpress.org/trunk@53404
Fixes #40736. Allows to register patterns on
current_screenoradmin_initagain, and these are sent serialized, as asettingsargument ofinitializeEditor, in the editor page HTML. Patterns from REST (preferred source from now) are added on top of these, instead of replacing them completely.