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

Localization of string literals (of the core) #44

Closed
sr258 opened this issue Aug 1, 2019 · 6 comments · Fixed by #363
Closed

Localization of string literals (of the core) #44

sr258 opened this issue Aug 1, 2019 · 6 comments · Fixed by #363

Comments

@sr258
Copy link
Member

sr258 commented Aug 1, 2019

While working on the validation part of h5p-extractor I've come across the need to use string literals that are displayed to the user (the validation errors). These literals should obviously be localized to the user's language.

The PHP library does it this way: Whenever it outputs a message to the user, it calls the function t($message, $replacements) with the English message string as first parameter. This function returns the translated string and does some placeholder replacements.

This means that the English message string is put in as a literal value into the actual source code. I think this is bad style (changing English literals means you have to look through the whole source code; creating translations is tedious; translations can't take context into consideration, which means that the same English literal in different places always has to have the same translation) and shouldn't be replicated in the NodeJS library.

What I've done is put the (English) literals into a JSON file as key-value pairs. The JSON file for the user's language is loaded by the framework and the content is passed to the validator as an object. Whenever the validator outputs a literal it looks up the string in the translation object through the key and outputs the value (the translation). If it requires placeholder replacements it does it with a little helper function (see here for an example).

I think my current approach is not the final word on localization of the NodeJS implementation (doesn't cater for single strings that are not translated in a language even though there is a language file for it), but this is something that has to be used project wide, so I would like to check if you've had any thoughts on this.

@sr258
Copy link
Member Author

sr258 commented Aug 2, 2019

I've gone ahead and introduced a mockup TranslationService in the validator. It can be injected in the constructor and implemented by every integration. If you like this approach I would extend it to the ContentTypeCache I've already written.

The more I look at it, the less sense it makes to have h5p-extractor as a standalone project. There will always be dependencies that have to be injected by h5p-editor-nodejs-library, so it would make sense to move extraction and validation to the editor.

@JPSchellenberg
Copy link
Member

I like your idea of a TranslationService and would say we use it. We can also integrate the H5P-Extractor into the Editor as they really seem to be tightly coupled. We definitely should try to avoid mistakes that the PHP-Library does like message strings within the source code.

@sr258
Copy link
Member Author

sr258 commented Aug 22, 2019

A question that has been bothering me for some time is: Does it make sense to call the translation service at the place where an error occurs or would it be better to simply report the error id and let the component that generates the error to the translation?

Advantages & disadvantages of translating at the position an error occurs:

  • Debugging is easier because the error is human readable.
  • You pass back error strings. If there are placeholders/variables in a string these already filled in.
  • Every component that does error reporting needs to have a reference to the translation service (which should not be global to allow proper unit testing) and be aware of how it works.

Advantages & disadvantages of translating somewhere at a very high level where errors are caught:

  • Debugging becomes less intuitive because you just see error ids and not proper texts.
  • Placeholders/variables have to be passed along with the error id.
  • There are few components that have to have a reference to the translation service.

What do you think?

@rtens
Copy link
Contributor

rtens commented Aug 23, 2019

My gut feeling is that translation is a presentation concern and should therefore be handled in the outer layer. We could use self-describing error IDs to make debugging easier.

@sr258
Copy link
Member Author

sr258 commented Sep 1, 2019

#80 Has to be done once the discussion is settled.

@JPSchellenberg
Copy link
Member

🎉 This issue has been resolved in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants