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

Fix concatenation of translation strings #554

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

pedro-mendonca
Copy link
Contributor

Use createInterpolateElement to make complete translation strings without concatenation.

Fixes #553

Use createInterpolateElement to make complete translation strings without concatenation.
Copy link
Contributor

@creativecoder creativecoder left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this translation bug!

My understanding is that the standard way to solve this is to use [sprintf from the @wordpress/i18n package], and use placeholders for any markup within the string. (https://github.com/WordPress/gutenberg/tree/trunk/packages/i18n#sprintf).

@creativecoder
Copy link
Contributor

My understanding is that the standard way to solve this is to use [sprintf from the @wordpress/i18n package]

Now I see from #553 that you already tried that! Looks like using createInterpolateElement is the pattern that's used in Gutenberg for this situation.

Copy link
Contributor

@creativecoder creativecoder 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 see the links correctly generated in wp-admin. Thanks for addressing this.

{
a: (
// eslint-disable-next-line jsx-a11y/anchor-has-content
<a href="/wp-admin/site-editor.php?canvas=edit" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Using an absolute path like this will break when using non-standard hosting configurations, like putting core files in a subdirectory, or subdirectory multisite installations.

But that's not a new problem with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've kept the text and link structure, only improved the i18n of the string.
Feel free to add any changes to the links in the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I think we should handle the link structure change separately from this PR.

@pbking
Copy link
Contributor

pbking commented Apr 18, 2024

This view has been depreciated and likely to be removed on the next release. However the work is already done and might be helpful as prior art somewhere. Bringing this in now.

@pbking pbking merged commit e70f992 into WordPress:trunk Apr 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[i18n] Translation strings with hardcoded concatenation
3 participants