-
Notifications
You must be signed in to change notification settings - Fork 46
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
Conversation
Use createInterpolateElement to make complete translation strings without concatenation.
There was a problem hiding this 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).
Now I see from #553 that you already tried that! Looks like using |
There was a problem hiding this 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" /> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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. |
Use createInterpolateElement to make complete translation strings without concatenation.
Fixes #553