Skip to content

More internationalization#456

Merged
gadenbuie merged 35 commits intorstudio:masterfrom
gadenbuie:more-i18n
Jan 15, 2021
Merged

More internationalization#456
gadenbuie merged 35 commits intorstudio:masterfrom
gadenbuie:more-i18n

Conversation

@gadenbuie
Copy link
Member

This PR builds on #441 (and includes the history in that branch) to extend the internationalization features and to expose the configuration options to the user for customization at a per-tutorial basis.

The usage and new format of the language option is outlined in the updated multilang vignette. There are quite a few different possibilities for the YAML options, ranging from simply changing the base language of the tutorial

language: fr

to overriding the display text for a specific language

# customize English display text for specific elements
language:
  default: en
  custom:
    button:
      runcode: Run!
    text:
      areyousure: Are you certain?

or to override the display text for multiple languages

language:
  default: en
  custom:
    - language: en
      button:
        runcode: Run!
    - language: es
      button:
        runcode: Ejecutar

or to provide the customizations in a JSON file

language:
  default: en
  custom: custom-language.json

Technical Changes

I've made a few changes compared with the approach in #441 that I'll outline briefly.

The first is that complete translations are now in R in tutorial_i18n_translations(). I think this will make it easier for R users to contribute more translations.

tutorial_i18n_lang() now receives the entire language list item from learnr::tutorial(), which is processed into a resources list as expected by i18next in tutorial_i18n_process_language_options() and written into the .js file containing the initialization code by tutorial_i18n_prepare_language_file(). Finally tutorial_i18n_lang() returns the htmlDependency() for the i18next initialization which contains the user's customizations.

Previously, a small amount of work was done to not re-create the initialization dependency file. Because that file can now depend on another external file (the custom-language.json file), I didn't really see any performance benefit to avoiding re-writing the file, so we now just write it on every render.

Users now have the multilang vignette and the documentation from tutorial_i18n_custom_language() to learn about language customization options. The latter is exported to facilitate creating the JSON file and to perform minimal validation of the custom language options.

Because the translations have been moved into R functions, the function documentation and the vignette both update automatically, so new translations can be added by updating tutorial_i18n_translations() without needing to update documentation in multiple locations.

@gadenbuie

This comment has been minimized.

Internationalization added an inner <span> to the button elements, so we need to check the parent button element in case the event is fired from the child <span>. The delegateTarget property is jQuery's currentTarget equivalent and has slightly better browser support.
@gadenbuie gadenbuie requested a review from schloerke December 29, 2020 20:17
@gadenbuie
Copy link
Member Author

language:
  en:
    button:
      runcode: Run!
    text:
      areyousure: Are you certain?
  es:
    button:
      runcode: Ejecutar
  fr: fr-translation.json

Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

Implement this change: #456 (comment)

@schloerke
Copy link
Collaborator

Let's also convert to htmldependencies

- Simplify structure of language arg
- Remove need for template file
- Use i18n_ prefix for all related functions
- Update multilang vignette
- Allow .json for all languages or just one
- Update tests
- button.questionsubmit and button.questiontryagain
- localizes after UI output changes
@gadenbuie gadenbuie requested a review from schloerke January 12, 2021 18:46
Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

Needs a news entry

@gadenbuie gadenbuie requested a review from schloerke January 13, 2021 22:36
@gadenbuie
Copy link
Member Author

A final thought: we might consider updating the documentation build process to also pull in vignettes, similar to pkgdown, so that the multilang vignette can be hosted online as well.

Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

LGTM given exercise_plural can be utilized

@gadenbuie
Copy link
Member Author

Fixes #108
Fixes #388
Closes #441

@gadenbuie

This comment has been minimized.

…t namespace

For historical reasons, i18next falls back to a language _within_ a namespace, before falling back to a namespace within the same language.

See i18next issue 1260
@gadenbuie
Copy link
Member Author

gadenbuie commented Jan 15, 2021

@schloerke Can you give the last change a quick look? I realized a subtle detail in the way i18next handles fallback languages and namespaces. For historical reasons, if a key is missing in a namespace, i18next falls back to the fallback language key in that namespace before it falls back to the same key in the original language's namespace.

As an example:

en:
  custom:
    button.runcode: 'Run'
es:
  translation:
    button.runcode: 'Ejecutar'

I was setting the default namespaces as custom and falling back to translation. But when looking for the run code in "es" i18next would use

es:custom:button.runcode || en:custom:button.runcode || es:translation:button.runcode || ...

which is not ideal.

The fix I've added is to keep the same custom and translation namespaces in learnr and in the object we serialized into the HTML. But then, before initializing i18next, I merge the custom object into the translation object, and then only use the translation namespace. This effectively reorders the above fallback path to

es:custom:... || es:translation:... || en:custom:... || en:translation:...

@gadenbuie gadenbuie requested a review from schloerke January 15, 2021 19:55
Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

LGTM given small JS change

Co-authored-by: Barret Schloerke <[email protected]>
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