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

More internationalization #456

Merged
merged 35 commits into from
Jan 15, 2021
Merged

More internationalization #456

merged 35 commits into from
Jan 15, 2021

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
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
R/html-dependencies.R Outdated Show resolved Hide resolved
R/html-dependencies.R Show resolved Hide resolved
R/quiz.R Show resolved Hide resolved
R/tutorial-format.R Outdated Show resolved Hide resolved
inst/lib/tutorial/tutorial.js Show resolved Hide resolved
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
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:...

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 <barret@rstudio.com>
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.

None yet

3 participants