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

Fonts Library: Fix translatable strings and enforce l10n best practices #58201

Closed
afercia opened this issue Jan 24, 2024 · 5 comments · Fixed by #58256
Closed

Fonts Library: Fix translatable strings and enforce l10n best practices #58201

afercia opened this issue Jan 24, 2024 · 5 comments · Fixed by #58256
Assignees
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs l10n Localization and translations best practices [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jan 24, 2024

Description

Splitting this out from #58110 / #58117

There's a couple places in the Fonts Library user interfaces where translatable strings are less than ideal and are hardly translatable:

<Text className="font-library-modal__font-card__count">
{ variantsText ||
variantsCount +
' ' +
_n( 'variant', 'variants', variantsCount ) }
</Text>

and

const supportedFormats =
ALLOWED_FILE_EXTENSIONS.slice( 0, -1 )
.map( ( extension ) => `.${ extension }` )
.join( ', ' ) +
` ${ __( 'and' ) } .${ ALLOWED_FILE_EXTENSIONS.slice( -1 ) }`;

To recap some of the l10n best practices, which are better detailed in the WordPress handbook:

  • Translatable strings must not use concatenation.
  • Always provide a full translatable string, do not split it in separate parts when possible.
  • Use sprintf() and placeholders when appropriate.
  • Don't ever include variables that refers to terms with varying genre / number within a single __() function. These cases need to be handled either bu providing separate strings or by using the _n() function.

Step-by-step reproduction instructions

N/A
See code snippets linked above.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Feature] Typography Font and typography-related issues and PRs l10n Localization and translations best practices [Feature] Font Library labels Jan 24, 2024
@afercia afercia changed the title Fonts library: FIx translatable strings and enforce l10n best practices Fonts Library: Fix translatable strings and enforce l10n best practices Jan 24, 2024
@afercia afercia self-assigned this Jan 24, 2024
@afercia
Copy link
Contributor Author

afercia commented Jan 24, 2024

The comma separator used in the code above .join( ', ' ) is an interesting case..

Such a separator should be translatable. Ideally, Gutenberg should implement something like WP_Locale->get_list_item_separator(). Cc @SergeyBiryukov for any thoughts and help (please).

@t-hamano
Copy link
Contributor

The comma separator used in the code above .join( ', ' ) is an interesting case..

This problem seems to exist throughout the Gutenberg project, not just the Font Library. If you search for join( ', ', you'll see that a similar approach is used to delimit list items embedded in strings in the block editor.

One approach might be to apply a translation function to each separator itself. In WordPress core, the wp_get_list_item_separator() function internally applies a translation function and returns a separator string.

https://github.com/WordPress/wordpress-develop/blob/fa441afd6e4bbe3aa6dbe9c43b590fb8e5f1c24d/src/wp-includes/l10n.php#L1904

@afercia
Copy link
Contributor Author

afercia commented Jan 25, 2024

@t-hamano yes that was one of my first thoughts. However, I'm not sure fixing this on a per case basis would be ideal.

wp_get_list_item_separator() was introduced in WordPress 6.0 to replace all the 'ad hoc' list item separators scattered all around through the codebase with a more abstracted, centralized solution. I think Gutenberg should take a similar approach.

Overall, I'd think Gutenberg should have more focus on localization issues as after almost 7 years of development it appears there's still lack of appropriate solutions and tools for l10n.

@t-hamano
Copy link
Contributor

If we add utility functions, it might be in the @wordpress/i18n package. However, I think it can be discussed in a separate issue, as it is a problem not limited to the Font Library.

@afercia
Copy link
Contributor Author

afercia commented Jan 25, 2024

I agree it's a broader issue that would need to be handled separately. It's more a l10n thing rather than a i19n one though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs l10n Localization and translations best practices [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants