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

Multiple languages: Change i18nId string to i18n object #1548

Conversation

hirokiterashima
Copy link
Member

@hirokiterashima hirokiterashima commented Dec 4, 2023

Notes

  • This will merge to the Multiple languages per unit #1513 feature branch (not to the develop branch)
  • The only file that changed is TranslateProjectService. Others are merge commits that for some reason still show up as being changed, even though the feature branch already contain those commits. I'm not quite sure how this happened.
  • Don't worry about styles in this PR

Changes

  • Change i18nId: string to i18n: { id: string, modified: number } in project.json. This will allow us to compare modified times of the original and translation strings and offer better translation support in the future.
  • Rename translation files from project.[locale].json to translations.[locale].json

Test Prep

  1. (Optional) make a backup of the project that has translations, in case you need to review other "Multiple Language..." PR's using the old i18n format and want to easily switch project versions
  2. In project.json, change instances of
   "some_key.i18nId": "abc123",

to

   "some_key.i18n": { "id": "abc123", "modified": 123 },
  1. Rename project.[locale].json to translations.[locale].json for all supported locales for the unit
  2. In translations.[locale].json, change instances of
   "abc123": "[translated text]",

to

   "abc123": {"value": "[translated text]", "modified": 345 },

Test

  • Switching between translations work as before. While logged in as a student or previewing a unit, test that:
    • Languages drop-down appears only if the unit has at least 1 supported locale
    • Switching between the language options immediately translates elements of the page that have translations
      • Test everywhere: unit plan, step select drop down, unit title
      • Elements that don't have translations appear in default locale
      • You can switch back to default locale

@hirokiterashima hirokiterashima added the enhancement New feature of any size or improvement (UI, performance, security) label Dec 4, 2023
@hirokiterashima hirokiterashima self-assigned this Dec 4, 2023
@hirokiterashima hirokiterashima marked this pull request as ready for review December 4, 2023 21:33
Copy link
Member

@geoffreykwan geoffreykwan left a comment

Choose a reason for hiding this comment

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

The new structure works well.

I noticed that if a translation value is an empty string, then an empty string will show up for the student. This can either be handled by checking for empty string or just making sure a translation entry is removed if a translator clears out a translation value. This is something we'll want to handle eventually.

@hirokiterashima
Copy link
Member Author

@geoffreykwan good point. I think we should ensure that we don't save empty translation values in the AT. I'll keep that in mind when working on the AT translation feature.

Copy link
Member

@breity breity left a comment

Choose a reason for hiding this comment

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

Looks good.

@hirokiterashima hirokiterashima merged commit 30e982c into issue-1513-multiple-languages-per-unit Dec 5, 2023
@hirokiterashima hirokiterashima deleted the multiple-languages-i18nId-string-to-i18n-object branch December 5, 2023 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature of any size or improvement (UI, performance, security)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants