bug 1525719: Extract strings from React, use existing translations #5302
Conversation
jwhitlock
force-pushed
the
react-puente-1525719
branch
from
1d2024a
to
cdb3121
Compare
|
I fixed a docker-only path in the Makefile, and added the missing |
This PR changes the Puente settings in kuma/settings/common.py so that Puente can find and extract gettext() strings in the React-based code in kuma/javascript/src/ Note that if we actually deploy this, we will cause work for localizers for things like the React header components that we've been using as test cases.
During development, the React views need all the strings from the 'javascript' domain, as well as some string that are currently in the 'django' domain. As we move toward production, strings may move as well, but for now there is a lot of duplication. Create a new 'react' domain that includes all the 'javascript' domain strings, as well as React-specific strings, most or all of which are repeated in the 'django' domain. Combine existing translations into a compendium to populate the 'react' domain. Load the strings JS using a new react_i18n function. This is paired with a change to the locale repo to .gitignore react.po and compendium files, until we're ready for independent translations.
* Removed ""Try it live!" string * Add new react.pot for front-end React code, should not appear as strings in Pontoon
React translations are currently populated from the Django and JavaScript translations. Grab the latest when the production image is built.
jwhitlock
force-pushed
the
react-puente-1525719
branch
from
71c005b
to
03eb41d
Compare
| @@ -81,10 +86,19 @@ localeextract: | |||
| python manage.py extract | |||
| python manage.py merge | |||
|
|
|||
| locale-populate-react: | |||
There was a problem hiding this comment.
It took me a while to figure out what this code is doing. Consider adding a comment here to explain the concatenation and extraction going on with msgcat and msgmerge.
It would also be useful to explain (because I'm not entirely clear) what the bottom line is here. I'm pretty sure I understand that this means that React components can use any string that is already in use by python or the existing static javascript files, and no new work will be created for translators. I'm not sure what happens if our React code starts using new strings... Will those end up on Pontoon for translation? Or will they just remain untranslated for now?
| @@ -81,10 +86,19 @@ localeextract: | |||
| python manage.py extract | |||
| python manage.py merge | |||
|
|
|||
| locale-populate-react: | |||
| @ mkdir -p locale/compendia | |||
There was a problem hiding this comment.
If you add an rmdir -rf at the end of this command, then you won't have to add that in the Dockerfile below... It doesn't look like the compendium files get used after they are created, so it is unclear to me that it is worth keeping them around. I suppose you could also use temp files to hold the compendia... But creating a special-purpose directory as you do here seems simpler to me.
| for localename in `find locale -mindepth 1 -maxdepth 1 -type d | cut -d/ -f2 | grep -v templates | grep -v compendia`; do \ | ||
| rm -f locale/compendia/$$localename.compendium; \ | ||
| msgcat --use-first -o locale/compendia/$$localename.compendium locale/$$localename/LC_MESSAGES/django.po locale/$$localename/LC_MESSAGES/javascript.po; \ | ||
| rm -f locale/$$localename/LC_MESSAGES/react.po; \ |
There was a problem hiding this comment.
If a React component had used a new string, is it possible that this react.po file being deleted here could contain a translation of that new string that does not appear in the compendium? If so, then this deletion might be a problem... Would it work to use this react.po (or a copy of it) instead of /dev/null in the msgmerge line below so that any React-only translations are not lost?
There was a problem hiding this comment.
This was more of a lesson for me than a review. It forced me to understand much more of our localization set-up/workflow than I did before. This looks great and worked for me locally. I have only one nit, mentioned below. Awesome work, thanks @jwhitlock!
| @@ -81,10 +86,19 @@ localeextract: | |||
| python manage.py extract | |||
| python manage.py merge | |||
|
|
|||
| locale-populate-react: | |||
| @ mkdir -p locale/compendia | |||
| for localename in `find locale -mindepth 1 -maxdepth 1 -type d | cut -d/ -f2 | grep -v templates | grep -v compendia`; do \ | |||
There was a problem hiding this comment.
For me, this find command returns the locale/.git directory as well, which in turn causes:
msgcat: error while opening "locale/.git/LC_MESSAGES/django.po" for reading: No such file or directory
msgmerge: error while opening "locale/compendia/.git.compendium" for reading: No such file or directory
when I run make locale-populate-react.
I modified the find command like this:
find locale -mindepth 1 -maxdepth 1 -type d -not -path locale/.git
and that got rid of the issue, but you may know a more elegant way to exclude the locale/.git directory.
|
I can add a comment explaining more about this code, and I'll try out moving the directory removal into the command. It may be next week. I intend this to be a temporary step until the React-based code is used in production. During this beta period, When we're ready to ship this in production, I expect that strings will move from the When we're ready to go live, I expect to run this command one last time to populate |
This builds on PR #5279, creating a new translations domain
reactthat includes all of thejavascriptstrings, as well as the React strings that are very likely to appear indjango.This requires the changes in mozilla-l10n/mdn-l10n@bfc4155, which include an updated
.gitignore. This was merged to master with the latest push to production.make localerefreshnow calls the new targetmake locale-populate-react, which collects existing translations into compendium files, which are used to populatereact.pofrom existing translations, without sending them to Pontoon.make build-staticnow calls the new targetmake compile-react-i18n, which works the same asmake compilejsi18n, but for the newreactdomain. A new Jinja2 commandreact_i18nreturns the URL of thesereact-domain JS files.To get things started, run this in the web container:
You can then see the translated strings in the React app, such as the French Learn page: