Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained. #45819

janusqa · 2022-11-16T18:21:34Z

Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained

What?

The action "Convert to regular blocks" is in plural form for all cases, including when the reusable block only contains a single block. It would be great if we detect the number of blocks contained and pluralize the message:

Reusable block only contains a single block: Convert to a regular block
Reusable block contains multiple blocks: Convert to regular blocks

This PR fixes the issue by properly pluralizing the label where necessary.

Why?

It resolves #34749. The action "Convert to regular blocks" should be in singular if the reusable block only contains a single block.

How?

Based on @fluiddot advice the block count was determined by checking the inner blocks rendered within a reusable block, and the count was use to determine if to pluralize the label.

Testing Instructions

Go to the web version of the editor.
Open a post/page.
Add any block and click on the options button located in the toolbar (the three dots icon).
Click on "Add to Reusable blocks" and set a name for the block.
Open the app (native version of the editor).
Open a post/page.
Tap on the heavy_plus_sign button.
Navigate to the Reusable tab.
Tap on the recently created reusable block and select the block.
Tap on the block settings (three dots button).
Observe that the action "Convert to regular blocks" has correct plural form depending on the number of blocks in the reusable block.

Screenshots or screencast

codesandbox · 2022-11-16T18:21:35Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

github-actions · 2022-11-16T18:22:02Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @janusqa! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

packages/block-library/src/block/edit.js

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

packages/block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js

Mamaduka · 2022-11-18T12:00:55Z

packages/block-library/src/block/edit.js

+	const innerBlockCount = useSelect(
+		( select ) => select( blockEditorStore ).getBlockCount( clientId ),
+		[ clientId ]
+	);


Since we're already selecting data from the block editor store, I would recommend combining these selectors. There's nothing wrong with using multiple useSelect in components, but in this case, I think it makes sense to fetch data using a single selector.

Here's an example:

const { canRemove, innerBlockCount } = useSelect( ( select ) => { const { canRemoveBlock, getBlockCount } = select( blockEditorStore ); return { canRemove: canRemoveBlock( clientId ), innerBlockCount: getBlockCount( clientId ), }; }, [ clientId ] );

@Mamaduka agreed. Will combine them in the other files as well if there is an per-existing select.

Mamaduka · 2022-11-18T12:02:29Z

packages/block-library/src/block/edit.native.js

+	const innerBlockCount = useSelect(
+		( select ) => select( blockEditorStore ).getBlockCount( clientId ),
+		[ clientId ]
+	);


Same as https://github.com/WordPress/gutenberg/pull/45819/files#r1026355761, and for the .native.js files as well, whenever applicable.

Thanks, I'll do this.

fluiddot

I tested the changes in the WordPress app and work as expected, thank you @janusqa for introducing this enhancement 🙇.

However, I identified a couple of places that we'd also need to pluralize the message:

Could we also update them? Thanks!

janusqa · 2022-11-28T13:23:28Z

I tested the changes in the WordPress app and work as expected, thank you @janusqa for introducing this enhancement bow.

However, I identified a couple of places that we'd also need to pluralize the message:
* https://github.com/WordPress/gutenberg/blob/5f0661c3f5718c10751b4d4ea26e00ab56e5c4cc/packages/block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js#L204-L206

* https://github.com/WordPress/gutenberg/blob/5f0661c3f5718c10751b4d4ea26e00ab56e5c4cc/packages/block-library/src/block/edit.native.js#L131-L132
Could we also update them? Thanks!

@fluiddot yes we can, will work on this. Thanks for your feedback.

janusqa · 2022-11-29T12:31:30Z

I tested the changes in the WordPress app and work as expected, thank you @janusqa for introducing this enhancement bow.
However, I identified a couple of places that we'd also need to pluralize the message:
* https://github.com/WordPress/gutenberg/blob/5f0661c3f5718c10751b4d4ea26e00ab56e5c4cc/packages/block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js#L204-L206

* https://github.com/WordPress/gutenberg/blob/5f0661c3f5718c10751b4d4ea26e00ab56e5c4cc/packages/block-library/src/block/edit.native.js#L131-L132

Could we also update them? Thanks!

@fluiddot yes we can, will work on this. Thanks for your feedback.

@fluiddot do you have a preference in the method used?

//method 1
const successNotice =
    innerBlockCount > 1
        ? 'converted to regular blocks'
        : 'converted to regular block';
createSuccessNotice(
    sprintf(
        /* translators: %1$s: name of the reusable block, %2$s: pluralized success message*/
        __('%1$s %2$s'),
        title,
        successNotice
    )
);

// method 2
createSuccessNotice(
    sprintf(
        /* translators: %s: block title */
        _n(
            '%s converted to regular block',
            '%s converted to regular blocks',
            innerBlockCount
        ),
        title
    )
);

fluiddot · 2022-11-29T13:24:26Z

@janusqa I'd lean towards method 1 in order to properly extract the strings for translation in mobile, as we don't support yet plurals in the extraction process to include them in the apps translation pipeline. Alternatively, I'd like to share a third option where each string is referenced in its own __ call, this option will generate two strings to translate instead of three:

Convert to regular blocks
Convert to regular block
%1$s %2$s

Third option:

const successNotice =
	innerBlockCount > 1
		? /* translators: %s: name of the reusable block */
			__( '%s converted to regular blocks' )
		: /* translators: %s: name of the reusable block */
			__( '%s converted to regular block' );
createSuccessNotice(
	sprintf( successNotice, reusableBlock?.title?.raw || blockTitle )
);

janusqa · 2022-11-29T13:32:36Z

@janusqa I'd lean towards method 1 in order to properly extract the strings for translation in mobile, as we don't support yet plurals in the extraction process to include them in the apps translation pipeline. Alternatively, I'd like to share a third option where each string is referenced in its own __ call, this option will generate two strings to translate instead of three:
* `Convert to regular blocks`

* `Convert to regular block`

* `%1$s %2$s`
Third option:
const successNotice =
	innerBlockCount > 1
		? /* translators: %s: name of the reusable block */
			__( '%s converted to regular blocks' )
		: /* translators: %s: name of the reusable block */
			__( '%s converted to regular block' );
createSuccessNotice(
	sprintf( successNotice, reusableBlock?.title?.raw || blockTitle )
);

Well well well, nicely done. this is very clean. I will use this one indeed.

janusqa · 2022-11-29T23:17:31Z

@fluiddot
Although the changes are made we may need to tweak three (3) places in reusable-blocks.test.js

    await clickBlockToolbarButton( 'Convert to regular blocks' )

It looks like we will need to tweak these test to look for the correct string depending on what the test creates as a reusable block.

fluiddot · 2022-11-30T09:29:31Z

@fluiddot Although the changes are made we may need to tweak three (3) places in reusable-blocks.test.js
    await clickBlockToolbarButton( 'Convert to regular blocks' )
It looks like we will need to tweak these test to look for the correct string depending on what the test creates as a reusable block.

@janusqa Good point, in fact, that's the reason why the End-to-End Tests / Admin - 3 check is failing in this PR. As you mentioned, we definitely need to update those tests and use the proper string in each case depending on if the Reusable block has one or more blocks 👍 . Would you be able to update it? Thanks!

…ks to regular blocks

… block conversion, to be more clean and efficient by reducing the number of string translations needed from three to two

…gular form

janusqa · 2022-11-30T16:26:35Z

@fluiddot Although the changes are made we may need to tweak three (3) places in reusable-blocks.test.js
    await clickBlockToolbarButton( 'Convert to regular blocks' )
It looks like we will need to tweak these test to look for the correct string depending on what the test creates as a reusable block.
@janusqa Good point, in fact, that's the reason why the End-to-End Tests / Admin - 3 check is failing in this PR. As you mentioned, we definitely need to update those tests and use the proper string in each case depending on if the Reusable block has one or more blocks +1 . Would you be able to update it? Thanks!

@fluiddot Yes I should be able to. Test updated. Used a simplistic approach based on the assumption that we know the type of reusable block we will create in the test ahead of time. Let me know if we need something more dynamic.

fluiddot

LGTM 🎊 ! I've tested the changes in the mobile app and they work as expected, thanks @janusqa for this fix.

I also gave a quick look at the web version to verify that it also works there. However, @Mamaduka since you're also assigned as a reviewer, let me know if you'd like to review the PR before merging it. Thanks!

Mamaduka · 2022-12-02T11:01:06Z

Thanks for the ping, @fluiddot! Everything looks good to me 👍

Congrats on your first contribution to the Gutenberg project, @janusqa 🎉

github-actions · 2022-12-02T11:26:00Z

Congratulations on your first merged pull request, @janusqa! 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!

janusqa · 2022-12-02T13:14:17Z

@Mamaduka @fluiddot thank you both for your guidance and assistance!

…ending on the number of blocks contained. (WordPress#45819) * Properly pluralize 'Convert to regular blocks' label on menus * Properly pluralize 'Convert to regular blocks' label on menus * Properly pluralized the success message when converting reusable blocks to regular blocks * Refactored generation of pluralized success notification for resuable block conversion, to be more clean and efficient by reducing the number of string translations needed from three to two * Fixed two reusable block test to look for correct button label in singular form

janusqa requested a review from ajitbohra as a code owner November 16, 2022 18:21

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

Mamaduka reviewed Nov 17, 2022

View reviewed changes

packages/block-library/src/block/edit.js Outdated Show resolved Hide resolved

Mamaduka reviewed Nov 17, 2022

View reviewed changes

...s/reusable-blocks/src/components/reusable-blocks-menu-items/reusable-blocks-manage-button.js Outdated Show resolved Hide resolved

Mamaduka reviewed Nov 17, 2022

View reviewed changes

packages/block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js Outdated Show resolved Hide resolved

Mamaduka added [Type] Enhancement A suggestion for improvement. [Block] Block The "Reusable Block" Block labels Nov 17, 2022

janusqa requested review from spacedmonkey, ntwb, nerrad, ellatrix and tellthemachines as code owners November 17, 2022 23:22

janusqa force-pushed the fix/properly-pluralize-the-message-convert-to-regular-blocks branch from f580e63 to a7435eb Compare November 18, 2022 00:52

fluiddot self-requested a review November 18, 2022 09:45

Mamaduka reviewed Nov 18, 2022

View reviewed changes

janusqa force-pushed the fix/properly-pluralize-the-message-convert-to-regular-blocks branch from a7435eb to 5f0661c Compare November 18, 2022 16:34

janusqa requested review from Mamaduka and removed request for spacedmonkey, ntwb, ajitbohra, nerrad, ellatrix, tellthemachines and fluiddot November 22, 2022 11:07

Mamaduka requested a review from fluiddot November 22, 2022 11:40

fluiddot reviewed Nov 28, 2022

View reviewed changes

janusqa force-pushed the fix/properly-pluralize-the-message-convert-to-regular-blocks branch from 882847c to 73eb85c Compare November 29, 2022 20:14

janusqa added 5 commits November 30, 2022 15:52

Properly pluralize 'Convert to regular blocks' label on menus

9f725a8

Properly pluralize 'Convert to regular blocks' label on menus

daee8c6

Properly pluralized the success message when converting reusable bloc…

cf5dbb5

…ks to regular blocks

Refactored generation of pluralized success notification for resuable…

c03b2e9

… block conversion, to be more clean and efficient by reducing the number of string translations needed from three to two

Fixed two reusable block test to look for correct button label in sin…

7b55aa1

…gular form

janusqa force-pushed the fix/properly-pluralize-the-message-convert-to-regular-blocks branch from 5d86e36 to 7b55aa1 Compare November 30, 2022 15:53

fluiddot approved these changes Dec 2, 2022

View reviewed changes

fluiddot merged commit 9850eec into WordPress:trunk Dec 2, 2022

github-actions bot added this to the Gutenberg 14.8 milestone Dec 2, 2022

janusqa deleted the fix/properly-pluralize-the-message-convert-to-regular-blocks branch December 2, 2022 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained. #45819

Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained. #45819

janusqa commented Nov 16, 2022

codesandbox bot commented Nov 16, 2022

github-actions bot commented Nov 16, 2022

Mamaduka Nov 18, 2022

janusqa Nov 18, 2022

Mamaduka Nov 18, 2022

janusqa Nov 18, 2022

fluiddot left a comment

janusqa commented Nov 28, 2022

janusqa commented Nov 29, 2022

fluiddot commented Nov 29, 2022

janusqa commented Nov 29, 2022

janusqa commented Nov 29, 2022

fluiddot commented Nov 30, 2022

janusqa commented Nov 30, 2022

fluiddot left a comment

Mamaduka commented Dec 2, 2022

github-actions bot commented Dec 2, 2022

janusqa commented Dec 2, 2022

Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained. #45819

Reusable block: Pluralize the message "Convert to regular blocks" depending on the number of blocks contained. #45819

Conversation

janusqa commented Nov 16, 2022

What?

Why?

How?

Testing Instructions

Screenshots or screencast

codesandbox bot commented Nov 16, 2022

github-actions bot commented Nov 16, 2022

Mamaduka Nov 18, 2022

Choose a reason for hiding this comment

janusqa Nov 18, 2022

Choose a reason for hiding this comment

Mamaduka Nov 18, 2022

Choose a reason for hiding this comment

janusqa Nov 18, 2022

Choose a reason for hiding this comment

fluiddot left a comment

Choose a reason for hiding this comment

janusqa commented Nov 28, 2022

janusqa commented Nov 29, 2022

fluiddot commented Nov 29, 2022

janusqa commented Nov 29, 2022

janusqa commented Nov 29, 2022

fluiddot commented Nov 30, 2022

janusqa commented Nov 30, 2022

fluiddot left a comment

Choose a reason for hiding this comment

Mamaduka commented Dec 2, 2022

github-actions bot commented Dec 2, 2022

janusqa commented Dec 2, 2022