Skip to content
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

Reusable Blocks: Preload user permissions #15061

Merged
merged 2 commits into from
Apr 24, 2019
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 18, 2019

Previously: #12378

This pull request seeks to add the blocks permissions REST API request as a preloaded path, for two reasons:

  • We use preload data to avoid UI "flickering". Presently, without these changes, UI such as the "Add to Reusable Blocks" button in a block settings menu appears only after a brief delay, which is undesirable.
  • As described at Testing: Skip unreliable end-to-end tests #15059 (comment) , the immediate unavailability of permissions data may be a culprit in end-to-end test instability

It appears this was already added in #12378 and exists in WordPress 5.2 (WordPress/wordpress-develop@79a3abc), but should remain in the Gutenberg plugin until WordPress 5.2 is the minimum supported version. It could be considered as erroneously removed via #13569.

Testing Instructions:

Verify that when clicking the block settings menu for the first time in a page session, the "Add to Reusable Blocks" is shown immediately if the user has permissions to undertake this operation (i.e. there is no delay in its appearance).

Ensure unit tests pass:

npm run test-php

@aduth aduth added Framework Issues related to broader framework topics, especially as it relates to javascript [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) REST API Interaction Related to REST API labels Apr 18, 2019
@aduth aduth requested a review from noisysocks April 18, 2019 19:54
@aduth aduth force-pushed the add/preload-block-permissions branch from af879fc to 8d1f833 Compare April 18, 2019 19:59
@aduth
Copy link
Member Author

aduth commented Apr 18, 2019

Rebased in mind of the merge of #15059, and unskipped the affected test described in #15059 (comment) to reflect the stability improved by these changes.

@aduth
Copy link
Member Author

aduth commented Apr 18, 2019

For future consideration: The implementation of gutenberg_extend_block_editor_preload_paths could be dramatically simplified and less error-prone on considering duplicates once we can officially drop support for PHP 5.2.

diff --git a/lib/client-assets.php b/lib/client-assets.php
index d9fcc8105..cde1ae0fb 100644
--- a/lib/client-assets.php
+++ b/lib/client-assets.php
@@ -582,10 +582,8 @@ function gutenberg_extend_block_editor_preload_paths( $preload_paths ) {
 	 * This is present in WordPress 5.2 and should be removed from Gutenberg
 	 * once WordPress 5.2 is the minimum supported version.
 	 */
-	if ( ! in_array( array( '/wp/v2/blocks', 'OPTIONS' ), $preload_paths ) ) {
-		$preload_paths[] = array( '/wp/v2/blocks', 'OPTIONS' );
-	}
+	$preload_paths[] = array( '/wp/v2/blocks', 'OPTIONS' );
 
-	return $preload_paths;
+	return array_unique( $preload_paths, SORT_REGULAR );
 }
 add_filter( 'block_editor_preload_paths', 'gutenberg_extend_block_editor_preload_paths' );

The function array_unique() does not have a parameter "sort_flags" in PHP version 5.2.8 or earlier (PHPCompatibility.PHP.NewFunctionParameters.array_unique_sort_flagsFound)

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm it is preloaded properly and I can see the menu item immediately 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Framework Issues related to broader framework topics, especially as it relates to javascript REST API Interaction Related to REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants