Replace 'Remove from Reusable blocks' with 'Manage Reusable blocks'. #27026

wallstead · 2020-11-17T04:23:34Z

Description

Removed ReusableBlockDeleteButton component and added ReusableBlocksManageButton in its place.
On click, ReusableBlocksManageButton sends the user to the reusable blocks management screen.
Added '@wordpress/url' dependency to the package. This is used to build the url for the reusable blocks management screen.

How has this been tested?

Clicked the button when there were no changes in the post, successfully sent to reusable blocks management screen.
Clicked the button when there were changes in the post, a warning is shown saying there are unsaved changes.
Ran npm test.

Screenshots

Types of changes

Replaces the 'Remove from Reusable blocks' button in block settings menu with a 'Manage Reusable blocks' button that takes the user to the reusable blocks management screen.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

talldan · 2020-11-17T07:30:14Z

packages/reusable-blocks/package.json

@@ -35,6 +35,7 @@
 		"@wordpress/i18n": "file:../i18n",
 		"@wordpress/icons": "file:../icons",
 		"@wordpress/notices": "file:../notices",
+		"@wordpress/url": "^2.19.0",


This dependency should be declared as a file link the same as the other @wordpress/ dependencies above.

talldan

Thanks so much for working on this @wallstead, the code looks really solid for your first contribution. I left a few comments, but nothing too complex.

talldan · 2020-11-17T07:43:27Z

...s/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-blocks-manage-button.js

+		( select ) => {
+			const { getBlock } = select( 'core/block-editor' );
+			const { canUser } = select( 'core' );
+			const blockObj = getBlock( clientId );


Would be great to avoid an abbreviation here (the coding standards says to use full names where possible - https://developer.wordpress.org/coding-standards/wordpress-coding-standards/javascript/#naming-conventions).

I think I'd go with naming this reusableBlock and change the later variable to reusableBlockPost (if it's needed).

talldan · 2020-11-17T07:47:49Z

...s/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-blocks-manage-button.js

+			const reusableBlock =
+				blockObj && isReusableBlock( blockObj )
+					? select( 'core' ).getEntityRecord(
+							'postType',
+							'wp_block',
+							blockObj.attributes.ref
+					  )
+					: null;


When I tested, blockObj.attributes.ref and reusableBlock.id were the same value, so just wondering if this call to getEntityRecord is needed? Is this to check that a post actually exists for the editor that's about to be loaded?

Worth noting that while the reusable block is saving I believe it has a temporary id (mentioned here - #19115), so we might want to check for that.

Good point. I don't think getEntityRecord is needed here. It was a remnant from the ReusableBlockDeleteButton component, but I don't think it is relevant for the user experience in this case. I removed it in my most recent commit.

As for the temporary id, I cannot replicate the ref attribute being anything other than a number id. I looked for documentation on this for a while but came up empty-handed.

talldan · 2020-11-17T07:51:22Z

...s/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-blocks-manage-button.js

+		<BlockSettingsMenuControls>
+			<MenuItem
+				onClick={ () => {
+					window.location = addQueryArgs( 'edit.php', {


I think this menu item should ideally render as a link (an <a> element instead of a <button>).

MenuItem can take the same props as Button, so it should be possible to pass the value to the href prop:
https://github.com/WordPress/gutenberg/blob/master/packages/components/src/button/index.js#L23

I do wonder if it should open in a new tab 🤔

Good to know this! I used your suggestion in my most recent commit.

Regarding the new tab functionality, I'm totally open to changing it but for now I'm leaving the functionality consistent with the "Manage all reusable blocks" button shown below. It doesn't open a new tab.

talldan

This is looking good! Thanks for making those few extra changes.

The only remaining thing I noticed is that the capitalization isn't consistent with the 'Manage all reusable blocks' option in the editor menu. Then I looked closer, and lots of strings seem inconsistent, so I made an issue (#27085).

I think 'Manage all reusable blocks' is the incorrect label here, so lets leave it as is.

If you can resolve the merge conflict I'd be happy to merge. Congrats on your first contribution!

* Removed ReusableBlockDeleteButton component and added ReusableBlocksManageButton in its place. * On click, ReusableBlocksManageButton sends the user to the reusable blocks management screen. * Added '@wordpress/url' dependency to the package. This is used to build the url for the reusable blocks management screen. Fixes WordPress#12791.

* Use file link for new @wordpress/url dependency. * Rename abbreviated blockObj variable to reusableBlock to follow coding standards. * Simplified logic to check if the 'Manage Reusable blocks' menu item should be visible. * Use MenuItem's href prop to create an a tag instead of relying on onClick on a button. Fixes WordPress#27026.

wallstead · 2020-11-19T19:48:16Z

@talldan I fixed the merge conflict and updated the package-lock to fix a failing check. Also, I saw a left-over e2e test for the "Remove from Reusable blocks" button we are replacing. I removed that unused test to fix another failing check. Hopefully, that's okay.

talldan · 2020-11-23T10:28:15Z

Nice work! Would be great to add an e2e test for the new 'Manage reusable blocks' option if you feel like taking a crack at it 😄

github-actions · 2020-11-23T10:29:00Z

Congratulations on your first merged pull request, @wallstead! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 17, 2020

talldan reviewed Nov 17, 2020

View reviewed changes

talldan approved these changes Nov 19, 2020

View reviewed changes

wallstead added 2 commits November 19, 2020 09:44

wallstead force-pushed the update/remove-from-reusable-blocks branch from e20c3e6 to ef3def1 Compare November 19, 2020 17:45

wallstead added 2 commits November 19, 2020 09:59

Update package lock. (WordPress#27026)

895fa90

Remove unused e2e test for deleting reusable blocks. (WordPress#27026)

9a42a9f

wallstead requested review from ajitbohra, nerrad and ntwb as code owners November 19, 2020 19:43

talldan merged commit 5a77049 into WordPress:master Nov 23, 2020

github-actions bot added this to the Gutenberg 9.5 milestone Nov 23, 2020

wallstead deleted the update/remove-from-reusable-blocks branch November 23, 2020 17:26

talldan added [Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Type] Enhancement A suggestion for improvement. labels Jan 21, 2021

ntsekouras mentioned this pull request Mar 1, 2021

Inconsistent string capitalization. #27085

Closed

Mamaduka mentioned this pull request Jan 19, 2022

Reusable block confirmation message is not translated #26714

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace 'Remove from Reusable blocks' with 'Manage Reusable blocks'. #27026

Replace 'Remove from Reusable blocks' with 'Manage Reusable blocks'. #27026

wallstead commented Nov 17, 2020

talldan Nov 17, 2020

talldan left a comment

talldan Nov 17, 2020

talldan Nov 17, 2020

wallstead Nov 19, 2020

talldan Nov 17, 2020

wallstead Nov 19, 2020

talldan left a comment

wallstead commented Nov 19, 2020

talldan commented Nov 23, 2020

github-actions bot commented Nov 23, 2020

Replace 'Remove from Reusable blocks' with 'Manage Reusable blocks'. #27026

Replace 'Remove from Reusable blocks' with 'Manage Reusable blocks'. #27026

Conversation

wallstead commented Nov 17, 2020

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

talldan Nov 17, 2020

Choose a reason for hiding this comment

talldan left a comment

Choose a reason for hiding this comment

talldan Nov 17, 2020

Choose a reason for hiding this comment

talldan Nov 17, 2020

Choose a reason for hiding this comment

wallstead Nov 19, 2020

Choose a reason for hiding this comment

talldan Nov 17, 2020

Choose a reason for hiding this comment

wallstead Nov 19, 2020

Choose a reason for hiding this comment

talldan left a comment

Choose a reason for hiding this comment

wallstead commented Nov 19, 2020

talldan commented Nov 23, 2020

github-actions bot commented Nov 23, 2020