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

Initial i18n setup #1428

Merged
merged 10 commits into from
Jul 17, 2024
Merged

Initial i18n setup #1428

merged 10 commits into from
Jul 17, 2024

Conversation

eric-gade
Copy link
Collaborator

@eric-gade eric-gade commented Jul 11, 2024

What does this PR do? 🛠️

This PR implements the recommendations in #1347 and is a follow-on to the ADR in #1427

What does the reviewer need to know? 🤔

Basic settings

With the enabling of the Drupal core Language and Interface Translation modules (reflected in the exported config), we now have the capability to localize the site in multiple languages. Those languages need to be added in the "Languages" part of the administration settings. For now, we only have one language enabled (English).

(NOTE: We have enabled English-English translations for now, as a way of testing our system. However, if English is set as the default language we can also turn off the need to provide translations for it later. For now it is beneficial to keep on until we officially start working in a second language).

The name of the specific setting for translating static assets from templates or other config files is "Interface Translation," and the config updates in this PR should also account for the correct settings there. Note: we do not want to update any individual phrase translations from within the CMS here, even though it offers that capability. We will strictly be using the .po files (see below) that are checked into this code repository.

Translation files

Drupal uses the gettext file format for specifying translations in each language.

The convention is to name each file with the two-letter code for the given language, followed by the gettext file extension of .po. So for English, our translation file is named en.po.

Gettext uses the convention where the English phrase is itself the key to other translations. For example, a portion of a Spanish translation file might look something like:

msgid "Hello"
msgstr "Hola"

Awkwardly, this means that our English translation file would have the same phrases as both the lookup and value. Fortunately, if we give a value of an empty string in these gettext files, the system will default to using the lookup key, which is already the English version of the phrase.

The initial translation files were generated first using the potx tool, followed by removing some extraneous pairs by hand.

New module weather_i18n

Following the example of the vote-gov team, our translation files as well as some i18n settings will be stored in a new custom module called weather_i18n. Translation files will be saved to web/modules/weather_i18n/translations.

We are using a custom module in part to have a regular place to store the translation files, but also so that any edge cases we encounter down the road can be addressed by module hooks or similar (ths is what happened with vote-gov).

Updating translations

If you edit a .po file, you will need to let Drupal know there are new translation updates, and also clear the cache. We have added a Makefile command make ut which will handle this for you.

Future work

We need to address testing the consistency of these files in #1429

Try it

If you want to test this for yourself, try the following:

  1. Pull down this branch, import the config, then clear cache or zap if needed
  2. Open the english translation file and add a new English-to-English translation for "Day" (ie, edit the msgstr value where the msgid is "Day").
  3. Run make ut
  4. Load any forecast page, then look at the daily forecast cards. Instead of saying "Day" for the daytime segment, it should now have the translated string you edited.

* Adding initial custom i18n module

-- Note
This module includes a po translations file for English to
English. The lack of translations for each term means the
default (the key, which is English already) will be used.

* The initial template file is custom edited, but was generated using
the potx tool.

* Export config

* Adding Makefile action for updating translations
Copy link
Contributor

It looks like our enabled modules has changed in this PR. Please ensure any documentation in https://github.com/weather-gov/weather.gov/tree/main/docs/dev/contributed-modules.md has been updated!

@eric-gade eric-gade marked this pull request as ready for review July 11, 2024 18:57
Comment on lines +83 to +85
docker compose exec drupal drush locale:clear-status
docker compose exec drupal drush locale:update
docker compose exec drupal drush cache:rebuild
Copy link
Member

Choose a reason for hiding this comment

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

will we want to add any of these steps to our post deploy script?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops! Yeah definitely, maybe that will help with my issues here...

@eric-gade eric-gade enabled auto-merge July 15, 2024 15:40
Copy link
Collaborator

@greg-does-weather greg-does-weather left a comment

Choose a reason for hiding this comment

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

Requesting one change, not covered in the comments below.

From the way the populate-database action was changed, I'm assuming that translations are loaded into the database from the PO files. If that's correct, then the action.yml files for populate-database and setup-site both need to be updated. They should add the PO files as inputs to the hash function used to create the cache key.

Eg.,

- name: database dump cache
  uses: actions/cache@v4
  id: db-cache
  with:
    key: drupal-database-${{ hashFiles('web/config/**/*.yml', 'web/scs-export/**/*') }}
    path: weathergov.sql

would become

```yaml
- name: database dump cache
  uses: actions/cache@v4
  id: db-cache
  with:
    key: drupal-database-${{ hashFiles('web/config/**/*.yml', 'web/scs-export/**/*', 'web/modules/weather_i18n/translations/*.po') }}
    path: weathergov.sql

#
msgid ""
msgstr ""
"Project-Id-Version: Vote.gov Utility\\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Project-Id-Version: Vote.gov Utility\\n"
"Project-Id-Version: weather.gov Utility\\n"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😅

web/modules/weather_i18n/translations/en.po Show resolved Hide resolved
@eric-gade
Copy link
Collaborator Author

Requesting one change, not covered in the comments below.
Whoa, good catch!

@eric-gade eric-gade merged commit 439518d into main Jul 17, 2024
17 checks passed
@eric-gade eric-gade deleted the eg-1347-i18n-initial-setup branch July 17, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants