Add support for translating strings in JavaScript files #3219

renatho · 2020-05-28T21:42:37Z

Changes proposed in this Pull Request

Update scripts to generate pot file with the texts from PHP and JS.
The pot file wasn't updated in this PR.

It was also introduced a script that adds the reference to the dist files in the pot file. The cons about that is for each new dist file, we have to add it to the additionalReferences array here (similar to this WooCommerce script):

sensei/bin/pot-dist-references.js

Lines 8 to 13 in b12c4b9

    
           const additionalReferences = [ 
        
           	{ 
        
           		re: /^#: assets\/onboarding\//gm, 
        
           		reference: '#: assets/dist/onboarding/index.js:1', 
        
           	}, 
        
           ];

At some point, we can think some alternative to update this file automatically while building our JS files.

How it works

Clean the temp files.
Generate a temporary pot file for the js(x) files.
Generate the PHP pot file, merging the JS pot file.
Add the dist references as an additional reference to each source occurrence. So the users can translate with the correct reference to the source in http://translate.wordpress.org/.
Clean the temp files.

Testing instructions

If you want to debug the whole process, you can temporarily remove the last npm run i18n:clean from the i18n:build script in the package.json.

Run npm run i18n:build
Check if the sensei-lms.pot was generated correctly.
Update some texts in the PHP and JS files (for example: assets/onboarding/index.jsx), rebuild, and make sure the sensei-lms.pot was updated correctly.
Remove some texts, rebuild, and make sure it was removed from the sensei-lms.pot file.

Testing the translations running in dev env:

Update the pot file: npm run i18n:build.
Create a translation po file with poedit or running msginit -l pt_BR -o sensei-lms-pt_BR.po in the lang folder.
If you generated through the command line, write some translations in the generated file.
Run msgfmt -o sensei-lms-pt_BR.mo sensei-lms-pt_BR.po to generate the mo file.
Run wp i18n make-json lang --no-purge to generate the Jed file for JS messages.
Move .mo and .json files to /wp-content/languages/plugins.
Go to WP-admin > Settings > General and update the language.
Access some JavaScript page (eg: /wp-admin/admin.php?page=sensei_onboarding) to test and check if the messages are translated.
Access some PHP page (eg: /wp-admin/edit.php?post_type=course) to test and check if the messages are translated.

Other observations:

The documentation wasn't updated yet. It will be done after the PR approval.
- With this PR, the main script to run the i18n build will be npm run i18n:build instead of gulp pot (which generates only the PHP part).
Our i18n build now depends on the wp i18n make-pot from the wp-cli.
As a bonus, while running the script wp i18n make-pot give us feedback about different translator comments that we can fix in a future PR.

References:

The wp-pot is restricted only to a PHP parser: https://github.com/wp-pot/wp-pot/blob/master/translation-parser.js#L5
~~We implemented the same idea than WooCommerce i18n script process~~: https://github.com/woocommerce/woocommerce-admin/blob/master/CONTRIBUTING.md#helper-scripts
Makepot plugin for Babel to generate the po from JS files: https://github.com/WordPress/gutenberg/tree/a1bc317086fca7602f03fcf88ea0c1de514656c3/packages/babel-plugin-makepot
Implementation details from a core committer, who looks very involved with the i18n part: https://pascalbirchler.com/internationalization-in-wordpress-5-0/
Demo project: https://github.com/swissspidy/gutenberg-i18n-block
Core class used for updating/installing language packs (Thanks @jom): https://developer.wordpress.org/reference/classes/language_pack_upgrader/
More details about JavaScript Internationalization:

yscik · 2020-05-28T22:32:58Z

Nice! One more aspect of this that's still needed to hook everything up is a wp_set_script_translations call to make the translations available for Javascript:
https://developer.wordpress.org/block-editor/developers/internationalization/

We should also test at some point that everything works with translate.wordpress.org.

donnapep · 2020-05-29T11:11:39Z

I noticed that the line that the JS text are referenced as being on is something like:

#: lang/tmp-sensei-js-texts.php:78
msgid "Join our mailing list"

Of course, there is no such file in the lang folder. Does it do this for WooCommerce too, or do they have a fix for that?

UPDATE: I checked this in WC admin and they reference the correct files:

#: src/API/Leaderboards.php:321
#: client/analytics/report/downloads/config.js:55
msgid "Product"
msgstr ""

yscik · 2020-05-29T16:12:22Z

includes/admin/class-sensei-onboarding.php

@@ -164,6 +173,7 @@ public function prepare_wizard_page() {
 		add_action( 'admin_print_scripts', [ $this, 'enqueue_scripts' ] );
 		add_action( 'admin_print_styles', [ $this, 'enqueue_styles' ] );
 		add_action( 'admin_body_class', [ $this, 'filter_body_class' ] );
+		add_action( 'init', [ $this, 'setup_wizard_set_script_translations' ] );


wp_set_script_translations will only work after the enqueue/register call, so instead of the init hook this should be in enqueue_scripts.

It was fixed here: e7c577e ;)

It must be done after the script register.

It didn't work well, duplicating some translations.

Now the i18n script is using wp i18n make-pot which also merges the JS pot file.

Borrowed from WooCommerce Admin plugin.

renatho · 2020-05-30T00:48:23Z

@donnapep and @yscik, it's almost ready!

We can already test following the instructions in the PR description. I just didn't understand yet what point in the WooCommerce Admin process they fix the source file references to the dist files. It's important because the JSON should be generated based in the file that our plugin is loading (the dist file).

Is there a process related to the translation that I don't know yet? It looks like WC folks generate the translated files after receiving the translations.

renatho · 2020-05-30T02:38:12Z

Rename the .jsx references to .js in the pot file. It's because the wp i18n make-json doesn't extract messages with jsx extensions. - The wp i18n make-json is used only for tests in our case, but I don't know how it will be parsed in WP.org. So to make sure that no problems will happen, we keep that. And I suspect it can be the reason to the WC folks use the React files with the .js extension.

Actually, I'm wondering if we should rename all .jsx to .js because if someone will translate, the reference link will be broken (with .js instead of .jsx). For our structure organization, I think it's negative, but for the translators, it would be better.

As WooCommerce and Guttenberg use the .js extension even for files with JSX content, I'm leaning to do the same.

Opinions about that?

donnapep · 2020-05-30T13:50:16Z

As WooCommerce and Guttenberg use the .js extension even for files with JSX content, I'm leaning to do the same.

For me, getting translations right is more important than .js vs .jsx naming.

UPDATE: See #3219 (comment).

~~I tried following the test instructions. At first I got the following error:~~
~~~ ~Warning: No WordPress install found. If the command 'i18n make-pot . lang/sensei-lms.pot' is in a plugin or theme, pass --path=`path/to/wordpress`.~ ~~~

~~Tried moving it to my local WordPress install and then got the following:~~
~ ~> wp i18n make-pot --merge=lang/tmp-sensei-lms-js.pot --skip-js --headers='{"Last-Translator":null,"Language-Team":null}' . lang/sensei-lms.pot~ ~~ ~Warning: mysqli_real_connect(): (HY000/2002): No such file or directory in /Users/donnapep/Local Sites/sensei/app/public/wp-includes/wp-db.php on line 1626 Error: Error establishing a database connection.~ ~

~~Tried starting the server but that didn't work either. The error is still there.~~

donnapep · 2020-05-30T13:59:25Z

OK, got it working. It's a bit of an inconvenience to have to build this in a WordPress directory now, but it's a small price to pay for working translations. We'll need to ensure this is added to the release instructions.

donnapep · 2020-05-30T14:06:39Z

Is there a process related to the translation that I don't know yet? It looks like WC folks generate the translated files after receiving the translations.

I'd suggest asking @rrennick about this to learn more about how the translation process works in WC admin.

renatho · 2020-05-31T00:39:23Z

OK, got it working. It's a bit of an inconvenience to have to build this in a WordPress directory now, but it's a small price to pay for working translations. We'll need to ensure this is added to the release instructions.

Actually I believe it's automatic and we need to do it only for tests.

Is there a process related to the translation that I don't know yet? It looks like WC folks generate the translated files after receiving the translations.

I'd suggest asking @rrennick about this to learn more about how the translation process works in WC admin.

Yeah! I sent him a message yesterday to understand how it works for them =)
When he answers, I update the situation here in the comments.

donnapep · 2020-06-01T11:52:12Z

Actually I believe it's automatic and we need to do it only for tests.

Do you mean that we're translating strings in test files? If so, is that necessary?

renatho · 2020-06-01T19:20:11Z

Actually I believe it's automatic and we need to do it only for tests.

Do you mean that we're translating strings in test files? If so, is that necessary?

Not for our automated tests, just for our user test to see if it would work (like simulating the translate.wordpress.org job).

Is there a process related to the translation that I don't know yet? It looks like WC folks generate the translated files after receiving the translations.

I'd suggest asking @rrennick about this to learn more about how the translation process works in WC admin.

Yeah! I sent him a message yesterday to understand how it works for them =)
When he answers, I update the situation here in the comments.

I talked to @rrennick and looks like there was a mistake in their last release while making the current pot file - and it's the reason why their pot file is pointing to the source files. But talking to him that we would like the translators see the correct reference (to the source files), he suggested we add the dist file as an additional reference, what IMHO makes sense, because this occurrences appear in the source and in the dist file. So, when generating the Jed files, it will generate the translation for the dist files.

renatho · 2020-06-01T19:21:40Z

We should also test at some point that everything works with translate.wordpress.org.

@yscik I think it would be the ideal. Do you know a way to do it?

donnapep · 2020-06-02T12:30:18Z

I think it would be the ideal. Do you know a way to do it?

I don't think we need to test this before merging. If there are issues we can open a new PR. Given this, can this PR be merged? The .pot file looks good to me.

renatho · 2020-06-02T12:56:18Z

I think it would be the ideal. Do you know a way to do it?

I don't think we need to test this before merging. If there are issues we can open a new PR. Given this, can this PR be merged? The .pot file looks good to me.

Yep! Just needing approval. After that, I'll update our release docs.

yscik

🥖

Add script to build pot file with PHP and JS texts

897c821

renatho added the [Type] Enhancement label May 28, 2020

renatho added this to the 3.1.0 milestone May 28, 2020

renatho requested a review from a team May 28, 2020 21:42

renatho self-assigned this May 28, 2020

donnapep added [Type] Maintenance and removed [Type] Enhancement labels May 29, 2020

Add script translations

ed1b01d

renatho force-pushed the add/js-pot-generator branch from 0402091 to ed1b01d Compare May 29, 2020 15:19

Add borrowed translation script from woocommerce-admin

9f70a2f

yscik reviewed May 29, 2020

View reviewed changes

renatho added 8 commits May 29, 2020 13:17

Update i18n scripts to keep the JS references

4db671a

Fix to generate JED for from JSX files

48805bf

Add script to replace jsx to js in the pot file

14ca399

Update i18n scripts

98c48f2

Fix how to set translation for JS files

e7c577e

It must be done after the script register.

Remove unused script

42301c7

It didn't work well, duplicating some translations.

Remove gulp i18n scripts

550d7a0

Now the i18n script is using wp i18n make-pot which also merges the JS pot file.

Add script to generate JSON files

715324a

Borrowed from WooCommerce Admin plugin.

Update pot scripts to add dist files as additional reference

b12c4b9

renatho requested review from yscik and donnapep June 1, 2020 19:20

yscik approved these changes Jun 2, 2020

View reviewed changes

renatho merged commit 955138a into master Jun 2, 2020

renatho deleted the add/js-pot-generator branch June 2, 2020 14:39

donnapep removed the [Type] Maintenance label Jun 2, 2020

donnapep changed the title ~~Add scripts to build pot file with PHP and JS texts~~ Add support for translating JavaScript files Jun 2, 2020

donnapep changed the title ~~Add support for translating JavaScript files~~ Add support for translating strings in JavaScript files Jun 2, 2020

jom mentioned this pull request Jun 23, 2020

Add support for translating strings in JavaScript files in Importer #3319

Merged

renatho mentioned this pull request Jul 31, 2020

Add a script to add automatically the dist references to the pot file #3493

Merged

This was referenced Oct 28, 2024

Missing translation texts for Sensei LMS Certificates (Block editor) #7693

Closed

Missing translation texts for Sensei LMS Certificates (Block editor) woocommerce/sensei-certificates#359

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for translating strings in JavaScript files #3219

Add support for translating strings in JavaScript files #3219

renatho commented May 28, 2020 •

edited

Loading

yscik commented May 28, 2020

donnapep commented May 29, 2020 •

edited

Loading

yscik May 29, 2020

renatho May 30, 2020

renatho commented May 30, 2020

renatho commented May 30, 2020 •

edited

Loading

donnapep commented May 30, 2020 •

edited

Loading

donnapep commented May 30, 2020

donnapep commented May 30, 2020

renatho commented May 31, 2020

donnapep commented Jun 1, 2020

renatho commented Jun 1, 2020

renatho commented Jun 1, 2020

donnapep commented Jun 2, 2020

renatho commented Jun 2, 2020

yscik left a comment

	const additionalReferences = [
	{
	re: /^#: assets\/onboarding\//gm,
	reference: '#: assets/dist/onboarding/index.js:1',
	},
	];

Add support for translating strings in JavaScript files #3219

Add support for translating strings in JavaScript files #3219

Conversation

renatho commented May 28, 2020 • edited Loading

Changes proposed in this Pull Request

How it works

Testing instructions

Testing the translations running in dev env:

Other observations:

References:

yscik commented May 28, 2020

donnapep commented May 29, 2020 • edited Loading

yscik May 29, 2020

Choose a reason for hiding this comment

renatho May 30, 2020

Choose a reason for hiding this comment

renatho commented May 30, 2020

renatho commented May 30, 2020 • edited Loading

donnapep commented May 30, 2020 • edited Loading

donnapep commented May 30, 2020

donnapep commented May 30, 2020

renatho commented May 31, 2020

donnapep commented Jun 1, 2020

renatho commented Jun 1, 2020

renatho commented Jun 1, 2020

donnapep commented Jun 2, 2020

renatho commented Jun 2, 2020

yscik left a comment

Choose a reason for hiding this comment

renatho commented May 28, 2020 •

edited

Loading

donnapep commented May 29, 2020 •

edited

Loading

renatho commented May 30, 2020 •

edited

Loading

donnapep commented May 30, 2020 •

edited

Loading