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

Playground block: Add i18n support #327

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Playground block: Add i18n support #327

merged 2 commits into from
Jul 16, 2024

Conversation

brandonpayton
Copy link
Member

What?

This PR adds i18n support to enable translation of Playground block strings.

Why?

Before this PR, Playground block strings were not rendered using i18n facilities and were not translatable.

How?

This PR uses @wordpress/i18n, adds a local i18n module with the correct text domain, and attempts to convert all fixed strings to using the i18n API.

Testing Instructions

Smoke test. Use the block in the editor and on the front end. Play with different block settings toggles, and ensure the user visible text is intact.

@akirk Somehow it doesn't appear possible to select you as a reviewer when creating this PR. Would you be willing to review this as well? I am relatively new to i18n APIs and would love to learn from your wisdom here.

@brandonpayton brandonpayton added Bug Something isn't working Playground block labels Jul 15, 2024
@brandonpayton brandonpayton self-assigned this Jul 15, 2024
{createInterpolateElement(
// translators: powered-by label with embedded icon. please leave markup tags intact, including numbering.
__(
'<span1>Powered by</span1> <Icon /> <span2>WordPress Playground</span2>'
Copy link
Member Author

Choose a reason for hiding this comment

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

The signature of createInterpolateElement() takes a map with keys for element names, so it is not possible to use regular <span> twice here and have them treated separately. But it turns out we can use unique non-existent tag names and achieve what we want.

It's not great because it might confuse translators who might "fix" our span "typos". To attempt to reduce the risk of this, I added a translators note that mentions the numbered markup tags.

@@ -0,0 +1,9 @@
import { createI18n, sprintf as coreSprintf } from '@wordpress/i18n';

const i18n = createI18n(undefined, 'interactive-code-block');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, I didn't know that you could set the domain in one place instead of writing it on every line.

Copy link
Member Author

@brandonpayton brandonpayton Jul 16, 2024

Choose a reason for hiding this comment

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

Nice, I didn't know that you could set the domain in one place instead of writing it on every line.

I didn't at first either. This PR started with manually passing text domain everywhere (and discovering that I'd forgotten to add it in a bunch of places). This is much better. ☺️

@brandonpayton brandonpayton merged commit 5b8542a into trunk Jul 16, 2024
2 checks passed
@brandonpayton brandonpayton deleted the add-i18n-support branch July 16, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Playground block
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants