Site Editor: Remote block patterns are not loaded

[noisysocks]

Originally reported in ​https://github.com/WordPress/gutenberg/issues/37814.

1. Activate an "empty" theme, e.g. the one that's in the Gutenberg repo. ​https://github.com/WordPress/gutenberg/tree/trunk/test/emptytheme
2. Open the Site Editor.
3. Open the inserter.
4. Select "Patterns".
5. Note that many block pattern categories are missing. (They appear in the post editor.)

The problem is that remote patterns are loaded when the current_screen hook is called.

​https://github.com/WordPress/wordpress-develop/blob/7f2ddd0ae6f128d4292d7cbd20d9e359bb176609/src/wp-includes/default-filters.php#L335

Before proceeding, this hook checks that $current_screen→is_block_editor is true.

​https://github.com/WordPress/wordpress-develop/blob/7f2ddd0ae6f128d4292d7cbd20d9e359bb176609/src/wp-includes/block-patterns.php#L55-L57

But is_block_editor is not set to true until after the current_screen hook is called.

​https://github.com/WordPress/wordpress-develop/blob/7f2ddd0ae6f128d4292d7cbd20d9e359bb176609/src/wp-admin/admin.php#L212

​https://github.com/WordPress/wordpress-develop/blob/7f2ddd0ae6f128d4292d7cbd20d9e359bb176609/src/wp-admin/site-editor.php#L32

The inserter hides the categories as they are empty.

Unfortunately there is no suitable hook that fires in between the call to $current_screen→is_block_editor( true ) and the call to WP_Block_Patterns_Registry::get_instance()->get_all_registered().

Perhaps we should call _load_remote_block_patterns and _load_remote_featured_patterns directly instead of using a hook? What do you think @hellofromtonya?

Load any remote block patterns from w.org explicitly instead of using the current_screen filter. The current_screen filter is unreliable as, in many cases e.g. the Site Editor, $current_scren->is_block_editor is not set until after the filter is executed.

Trac ticket: https://core.trac.wordpress.org/ticket/54806

[peterwilsoncc]

Related #46195

The previous fix was to add some logic to WP_Screen to improve the accuracy of block editor detection, see ​d575d70fe.

I'm in two minds about the proposed patch in ​the linked pull request as I worry it could lead to blocking HTTP requests during page load when patterns are not needed.

I'll dig in to the code further and see if my concern is valid.

[noisysocks]

I'm in two minds about the proposed patch in the linked pull request as I worry it could lead to blocking HTTP requests during page load when patterns are not needed.

Which screens don't need to load patterns? In my patch, I explicitly load them in the site editor (site-editor.php) and block editor (edit-form-blocks.php) which are the two places they're needed. I'm not loading them in the widgets block editor (widgets-form-blocks.php) as we don't support patterns there yet. There are no other calls to →is_block_editor( true ).

[peterwilsoncc]

I'm thinking about _load_remote_block_patterns() being called at an unexpectedly and relying on the is_block_editor check exists to prevent the request if it's not needed. For example, custom pages that use the block editor.

As it is, I don't think it's a problem, I just wanted to hold things up so I could check the plugin repo (​it's dandy) and order of operations on a few things.

[noisysocks]

I could keep the if ( ! $current_screen->is_block_editor ) check in _load_remote_block_patterns for backwards compatibly if we want. Just in case anybody is using it as it's not marked private.

[peterwilsoncc]

Replying to noisysocks:

I could keep the if ( ! $current_screen->is_block_editor ) check in _load_remote_block_patterns for backwards compatibly if we want. Just in case anybody is using it as it's not marked private.

Yeah, let's keep it there to avoid the minor break in back-compatibility.

[noisysocks]

👍 done.

[hellofromTonya]

Test Report: before (can reproduce ✅) and after PR 2178 (resolves issue ✅)

[hellofromTonya]

Test Report

Env:

• Theme: ​empty theme
• Plugins: none
• WordPress: trunk
• Localhost: wp-env
• Browsers: Firefox, Chrome, Edge, Safari
• OS: macOS Big Sur

Steps

Theme:

1. Download files from Gutenberg repo
2. Activate the empty theme

To test:

1. Go to Posts > All Posts and edit "Hello world!" post
2. Toggle open the inserter (click on the + icon next to the WordPress logo)
3. Click on the Patterns tab. Expected behavior: patterns are loaded into the UI and dropdown is populated for exploring
4. Go to Appearance > Editor (beta)
5. Repeat steps 2 and 3. Expected behavior: patterns are loaded into the UI and dropdown is populated for exploring

Test Results

• Without the PR
  • Patterns available in the block editor
  • Patterns not available in site editor (able to reproduce the reported issue ✅)
• With the PR applied:
  • Patterns available in the block editor
  • Patterns available in site editor (resolves reported issue ✅)

[hellofromTonya]

Capturing comments from my review of the PR:

• The changes apply to only the block editor and site editor screens 👍
• Function signature deprecation is consistent with other instances in Core 👍
• Backwards compatibility functionality in _load_remote_block_patterns() is retained 👍
• In testing, this PR resolves the reported issue (see the test report) while not impacting the block editor 👍

The solution works around the current_screen limitations to directly trigger remote pattern loading when on block or site editor screens. Should not impact other UIs/screens. I think this makes sense.

LGTM 👍 Marking for commit.

What do you think @peterwilsoncc?

[audrasjb]

I tested the proposed PR and it fixes the issue: patterns are available in both the block and site editors.

[peterwilsoncc]

Thanks @noisysocks, with the latest push the linked pull request looks commit ready to me.

Thanks for the gif and testing notes @hellofromTonya

[hellofromTonya]

Cool. Thanks @audrasjb and @peterwilsoncc for testing and reviewing. Let's it committed and backported w00t

Self-assigning for commit.

[hellofromTonya]

In 52593:

Editor: Explicitly load remote block patterns in the block and site editor screens.

Remote block patterns from wp.org were to be loaded through a callback hooked into the current_screen filter. Within 2 callbacks, i.e. _load_remote_featured_patterns() and _load_remote_block_patterns(), a guard clause bailed out early if the $current_screen->is_block_editor is false.

However, the current_screen filter is unreliable to detect the block editor. Why? In the block and Site Editor screens, $current_scren->is_block_editor is not set until after the filter is executed. Whoopsie.

This commit no longer uses the current_screen filter. Instead, it explicitly loads the remote block patterns by invoking both private functions (now not callbacks) directly in the screen files for the block and site editor screens.

With this change, passing WP_Screen object into these functions is no longer needed. As the _load_remote_block_patterns() function was introduced in 5.8.0, its function parameter is now deprecated and the guard clause retained for backwards compatibility.

Follow-up to [51021], [52377].

Props poena, noisysocks, peterwilsoncc, hellofromTonya, audrasjb.
Fixes #54806.

Committed via https://core.trac.wordpress.org/changeset/52593.

[hellofromTonya]

Reopening to backport [52593] to 5.9. Thank you @audrasjb for the 2nd committer review!

[hellofromTonya]

In 52594:

Editor: Explicitly load remote block patterns in the block and site editor screens.

Remote block patterns from wp.org were to be loaded through a callback hooked into the current_screen filter. Within 2 callbacks, i.e. _load_remote_featured_patterns() and _load_remote_block_patterns(), a guard clause bailed out early if the $current_screen->is_block_editor is false.

However, the current_screen filter is unreliable to detect the block editor. Why? In the block and Site Editor screens, $current_scren->is_block_editor is not set until after the filter is executed. Whoopsie.

This commit no longer uses the current_screen filter. Instead, it explicitly loads the remote block patterns by invoking both private functions (now not callbacks) directly in the screen files for the block and site editor screens.

With this change, passing WP_Screen object into these functions is no longer needed. As the _load_remote_block_patterns() function was introduced in 5.8.0, its function parameter is now deprecated and the guard clause retained for backwards compatibility.

Follow-up to [51021], [52377].

Props poena, noisysocks, peterwilsoncc, hellofromTonya, audrasjb.
Merges [52593] to the 5.9 branch.
Fixes #54806.

[hellofromTonya]

Alrighty, this has been committed to trunk and backported to 5.9 branch. Thank you everyone for your contributions and to @poena for reporting it.