Add ESLint rule to prevent usage of the verb 'toggle' in translatable strings

[im3dabasia]

Closes : #66372

What?

As described in the above issue, the Gutenberg repository moved away from using the verb "toggle" because it was causing issues with translations. A new issue was proposed (#66372) to add an ESLint rule to prevent contributors from using this word in translation-ready functions like __().

Why?

The usage of the verb "toggle" was causing incorrect translations in the core, as it does not translate well in many languages.

How?

An ESLint rule was added to prevent the usage of the verb "toggle."

Screenshots or screencast

Screen.Recording.2024-12-11.at.11.38.52.AM.mov

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: afercia <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[im3dabasia]

Hey @afercia,
Could you please review this PR when you have a moment?

[Mamaduka]

@swissspidy, I think you've got the most experience with custom i18n ESLint rules. Do you mind having a look at this?

[swissspidy]

IMHO such a rule is too niche/specific for this specific string. If anything, it should use a user-configurable blocklist. It's also very specific for WordPress/Gutenberg, so I don't think eslint-plugin is a good fit for it.

I would just use no-restricted-syntax for this. There's a lot of precedent for this in eslintrc.js:

gutenberg/.eslintrc.js

Lines 86 to 140 in 2ee383c

	const restrictedSyntax = [
	// NOTE: We can't include the forward slash in our regex or
	// we'll get a `SyntaxError` (Invalid regular expression: \ at end of pattern)
	// here. That's why we use \\u002F in the regexes below.
	{
	selector:
	'ImportDeclaration[source.value=/^@wordpress\\u002F.+\\u002F/]',
	message: 'Path access on WordPress dependencies is not allowed.',
	},
	{
	selector:
	'CallExpression[callee.name="deprecated"] Property[key.name="version"][value.value=/' +
	majorMinorRegExp +
	'/]',
	message:
	'Deprecated functions must be removed before releasing this version.',
	},
	{
	selector:
	'CallExpression[callee.object.name="page"][callee.property.name="waitFor"]',
	message:
	'This method is deprecated. You should use the more explicit API methods available.',
	},
	{
	selector:
	'CallExpression[callee.object.name="page"][callee.property.name="waitForTimeout"]',
	message: 'Prefer page.waitForSelector instead.',
	},
	{
	selector: 'JSXAttribute[name.name="id"][value.type="Literal"]',
	message:
	'Do not use string literals for IDs; use withInstanceId instead.',
	},
	{
	// Discourage the usage of `Math.random()` as it's a code smell
	// for UUID generation, for which we already have a higher-order
	// component: `withInstanceId`.
	selector:
	'CallExpression[callee.object.name="Math"][callee.property.name="random"]',
	message:
	'Do not use Math.random() to generate unique IDs; use withInstanceId instead. (If you’re not generating unique IDs: ignore this message.)',
	},
	{
	selector:
	'CallExpression[callee.name="withDispatch"] > :function > BlockStatement > :not(VariableDeclaration,ReturnStatement)',
	message:
	'withDispatch must return an object with consistent keys. Avoid performing logic in `mapDispatchToProps`.',
	},
	{
	selector:
	'LogicalExpression[operator="&&"][left.property.name="length"][right.type="JSXElement"]',
	message:
	'Avoid truthy checks on length property rendering, as zero length is rendered verbatim.',
	},
	];

The following rule works just fine:

	{
		selector: 'Literal[value=/^toggle/i]',
		message: "Avoid using the verb 'Toggle' in translatable strings",
	},

[im3dabasia]

Hi @swissspidy,

I used Literal[value=/^toggle\\b/i] to ensure that the rule only matches the word "toggle" when it appears standalone, excluding cases like "ToggleControl."

This approach helps avoid false positives in translatable strings.

Please take a look at my PR and let me know your feedback.

[swissspidy]

Looks much better! Now we just need to fix the 20 newly reported errors :)

[im3dabasia]

@swissspidy,

Upon reviewing the errors flagged by the current rule, it appears that the rule is raising errors for the word "toggle" whenever it appears in any string literal, including cases where it is not inside a translation function (such as in Examples 11, 12, 13, etc.).

However, our initial goal was to only prevent the use of the word "toggle" in translation-ready functions like __() and similar.

Could you please advise on how to adjust the rule to restrict the error only to those specific translation contexts, and prevent it from flagging usage of "toggle" in other non-translation-related string literals?

Thank you in advance for your guidance!

1. gutenberg/packages/block-editor/src/components/responsive-block-control/index.js

   Lines 59 to 61 in 287d653

   	const toggleHelpText = __(
   	'Toggle between using the same value for all screen sizes or using a unique value per screen size.'
   	);

2. gutenberg/packages/block-library/src/navigation/edit/test/navigation-menu-selector.js

   Lines 73 to 90 in 287d653

   	describe( 'NavigationMenuSelector', () => {
   	describe( 'Toggle', () => {
   	it( 'should show dropdown toggle with loading message when menus have not resolved', async () => {
   	useNavigationMenu.mockReturnValue( {
   	navigationMenus: [],
   	isResolvingNavigationMenus: true,
   	hasResolvedNavigationMenus: false,
   	canSwitchNavigationMenu: true,
   	} );
   	render( <NavigationMenuSelector /> );
   	expect(
   	screen.getByRole( 'button', {
   	name: /Loading/,
   	} )
   	).toBeInTheDocument();
   	} );

3. gutenberg/packages/block-library/src/query-pagination/query-pagination-label-control.js

   Lines 12 to 14 in 287d653

   	help={ __(
   	'Toggle off to hide the label text, e.g. "Next Page".'
   	) }

4. gutenberg/packages/block-library/src/table-of-contents/edit.js

   Lines 119 to 127 in 84fc197

   	help={
   	onlyIncludeCurrentPage
   	? __(
   	'Only including headings from the current page (if the post is paginated).'
   	)
   	: __(
   	'Toggle to only include headings from the current page (if the post is paginated).'
   	)
   	}

5. gutenberg/packages/components/src/composite/test/index.tsx

   Lines 175 to 177 in 84fc197

   	const toggleButton = screen.getByRole( 'button', {
   	name: 'Toggle third item',
   	} );

6. gutenberg/packages/components/src/date-time/date/test/use-lilius.ts

   Lines 319 to 320 in 84fc197

   	describe( 'toggle', () => {
   	it( 'toggles the selection of a date', () => {

7. gutenberg/packages/components/src/dropdown/test/index.tsx

   Lines 72 to 73 in 84fc197

   	await user.click( screen.getByRole( 'button', { name: 'Toggle' } ) );

8. gutenberg/packages/components/src/modal/test/index.tsx

   Lines 329 to 331 in 84fc197

   	const opener = screen.getByRole( 'button', {
   	name: 'Toggle Modal',
   	} );

9. gutenberg/packages/components/src/modal/test/index.tsx

   Lines 343 to 345 in 84fc197

   	const opener = screen.getByRole( 'button', {
   	name: 'Toggle Modal',
   	} );

10. gutenberg/packages/components/src/modal/test/index.tsx

    Lines 385 to 387 in 84fc197

    	const opener = screen.getByRole( 'button', {
    	name: 'Toggle Modal',
    	} );

11. gutenberg/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx

    Lines 56 to 59 in 84fc197

    	const id = useInstanceId(
    	ToggleGroupControlOptionBase,
    	toggleGroupControlContext.baseId || 'toggle-group-control-option-base'
    	);

12. gutenberg/packages/components/src/toggle-group-control/toggle-group-control/as-button-group.tsx

    Lines 38 to 41 in 84fc197

    	const generatedId = useInstanceId(
    	ToggleGroupControlAsButtonGroup,
    	'toggle-group-control-as-button-group'
    	);

13. gutenberg/packages/components/src/toggle-group-control/toggle-group-control/as-radio-group.tsx

    Lines 45 to 48 in 84fc197

    	const generatedId = useInstanceId(
    	ToggleGroupControlAsRadioGroup,
    	'toggle-group-control-as-radio-group'
    	);

14. gutenberg/packages/data/src/components/use-select/test/index.js

    Lines 129 to 139 in 84fc197

    	it( 'does not rerender a nested component that is to be unmounted', () => {
    	registry.registerStore( 'toggler', {
    	reducer: ( state = false, action ) =>
    	action.type === 'TOGGLE' ? ! state : state,
    	actions: {
    	toggle: () => ( { type: 'TOGGLE' } ),
    	},
    	selectors: {
    	get: ( state ) => state,
    	},
    	} );

15. gutenberg/packages/edit-post/src/components/preferences-modal/enable-custom-fields.js

    Lines 19 to 22 in 84fc197

    	function submitCustomFieldsForm() {
    	const customFieldsForm = document.getElementById(
    	'toggle-custom-fields-form'
    	);

16. gutenberg/packages/edit-post/src/components/preferences-modal/test/enable-custom-fields.js

    Lines 81 to 83 in 84fc197

    	expect( getElementById ).toHaveBeenCalledWith(
    	'toggle-custom-fields-form'
    	);

17. gutenberg/packages/editor/src/components/post-publish-button/post-publish-button-or-toggle.js

    Lines 13 to 14 in 84fc197

    	const IS_TOGGLE = 'toggle';
    	const IS_BUTTON = 'button';

18. gutenberg/packages/editor/src/components/post-publish-button/test/post-publish-button-or-toggle.js

    Lines 58 to 67 in 84fc197

    	it( 'toggle when post is not (1), (2), (3), the viewport is <= medium, and the publish sidebar is enabled', () => {
    	useViewportMatch.mockReturnValue( true );
    	useSelect.mockImplementation( () => ( {
    	isPublishSidebarEnabled: true,
    	} ) );
    	render( <PostPublishButtonOrToggle isPublishSidebarEnabled /> );
    	expect(
    	screen.getByRole( 'button', { name: 'Publish' } )
    	).toBeVisible();
    	} );

19. gutenberg/packages/token-list/src/test/index.ts

    Lines 243 to 252 in 84fc197

    	describe( 'toggle', () => {
    	describe( 'without force', () => {
    	it( 'toggles off', () => {
    	const list = new TokenList( 'abc' );
    	const returnValue = list.toggle( 'abc' );
    	expect( list.value ).toBe( '' );
    	expect( list ).toHaveLength( 0 );
    	expect( returnValue ).toBe( false );
    	} );

[swissspidy]

So how about CallExpression[callee.name=/^(__|_x|_n|_nx)$/] > Literal[value=/^toggle\b/i]

[im3dabasia]

Thank you very much, @swissspidy. I really appreciate it.

After adopting the new approach, the errors have been reduced to three, which are string literals in the __() functions.

I believe these might have been missed previously.

1. 'Toggle between using the same value for all screen sizes or using a unique value per screen size.'
2. 'Toggle off to hide the label text, e.g. "Next Page".'
3. 'Toggle to only include headings from the current page (if the post is paginated).'

Click to view code

gutenberg/packages/block-editor/src/components/responsive-block-control/index.js

Lines 59 to 61 in 287d653

	const toggleHelpText = __(
	'Toggle between using the same value for all screen sizes or using a unique value per screen size.'
	);

gutenberg/packages/block-library/src/query-pagination/query-pagination-label-control.js

Lines 12 to 14 in 287d653

	help={ __(
	'Toggle off to hide the label text, e.g. "Next Page".'
	) }

gutenberg/packages/block-library/src/table-of-contents/edit.js

Lines 119 to 127 in 84fc197

	help={
	onlyIncludeCurrentPage
	? __(
	'Only including headings from the current page (if the post is paginated).'
	)
	: __(
	'Toggle to only include headings from the current page (if the post is paginated).'
	)
	}

[swissspidy]

So can we fix them? :-)

[Mamaduka]

The Query Pagination text has a separate issue - #51335. I'm working on a fix - #68105.

[im3dabasia]

Could you please suggest which of the following revised strings would be the most appropriate?

1. Original:
   'Toggle between using the same value for all screen sizes or using a unique value per screen size.'

   Revised Alternatives:

   • 'Choose whether to use the same value for all screen sizes or a unique value for each screen size.'
   • 'Select whether to apply the same value across all screen sizes or a different value for each size.'

2. Original: (@Mamaduka is working on this)
   'Toggle off to hide the label text, e.g. "Next Page".'

   Revised Alternatives:

   • 'Disable to hide the label text, such as "Next Page".'
   • 'Turn off to hide the label text (e.g., "Next Page").'

3. Original:
   'Toggle to only include headings from the current page (if the post is paginated).'

   Revised Alternatives:

   • 'Select to include only headings from the current page (if the post is paginated). '
   • 'Enable to include only headings from the current page (if the post is paginated). '

[swissspidy]

I'd say:

• 'Choose whether to use the same value for all screen sizes or a unique value for each screen size.'
• Only include headings from the current page (if the post is paginated).

[Mamaduka]

@im3dabasia, the Query Pagination fix has been merged. Rebasing will resolve the failing Static Analysis check.

[im3dabasia]

@im3dabasia, the Query Pagination fix has been merged. Rebasing will resolve the failing Static Analysis check.

Thank you for the update. I will go ahead and rebase my branch right away