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

feat: i18n #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: i18n #56

wants to merge 1 commit into from

Conversation

lpetic
Copy link

@lpetic lpetic commented Mar 6, 2021

Hi,

I set up an i18n module.
There are no external dependencies.
I loaded 2 available languages (en & fr).

Principipaly this is based on (key, value) JSON file.
I inspire myself from the jQuery attribute data-i18n.

In the HTML file:

<h1 data-i18n="app"></h1>

In the JavaScript files:

displayError(trans.get('disconnected'), 'error');

Fix: #33

@jech
Copy link
Owner

jech commented Mar 7, 2021

This only translates messages that come from the Javascript, right? Or am I missing something and does it also translate the parts of the UI that are in HTML?

@lpetic
Copy link
Author

lpetic commented Mar 7, 2021

The translation is supported on both sides.

Only some error messages are not supported, because there are directly given by the server.

Enregistrement.de.l.ecran.2021-03-07.a.13.04.26.mov

@Mejans
Copy link

Mejans commented Mar 7, 2021

How Translation.prototype.selectLanguage = function (language) works?
Does it parse all the languages in the header accepted_language or only the first one?

@lpetic
Copy link
Author

lpetic commented Mar 7, 2021

I didn't use any header as Accepted-Language. Should I (maybe I missed something)? I touched only the client-side. The serve didn't know which language the user uses.

Translation.prototype.selectLanguage = function (language){
    this.language = this.languages.filter(l => l === language).length === 1 ? language : this.default;
}

This method will initialize the field this.language in the constructor. By default, I use the field navigator.language.

The method Translation.prototype.analyse will analyze the HTML and will set the right value related to the key stored by the attribute data-i18n.

@Mejans
Copy link

Mejans commented Mar 8, 2021

I meant, does it read all the languages a user has chosen or only the first one?
I don't see a loop and I don't understand turner things :S

@jech
Copy link
Owner

jech commented Mar 8, 2021

The patch uses the navigator.language value. As @inlor suggested, it should probably loop over navigator.languages (notice the s at the end) and pick the first language for which a translation is available, but that can be added in a future version.

For now, the important things to decide are:

  • is this feature worth the extra maintenance burden?
  • is this the right approach, or should we be using the English strings as indices in the translation table, as is the case with GNU gettext?

I have little experience with translation (I'm usually into software in English but with full support for non-ASCII scripts), so I'd welcome advice on both points above.

@lpetic
Copy link
Author

lpetic commented Mar 9, 2021

  • I don't think that this feature will grow up the maintainability (after all, there are only messages).
  • I saw examples here, to my mind usage of English string instead of key seems to be fragile. But, if you think that is better, I can change it @jech.

@Mejans seems to have some experience with i18n modules.

@Mejans
Copy link

Mejans commented Mar 9, 2021

I usually translate and some times I write code :)
And often I see some web sites only take the first language then falling back to English whereas the second language was available :(

@jech jech force-pushed the master branch 2 times, most recently from dd6d4c6 to b660bc4 Compare March 13, 2021 15:30
@lpetic
Copy link
Author

lpetic commented Apr 2, 2021

@jech Any news related to this PR?

@Mejans
Copy link

Mejans commented Apr 16, 2021

I'm coming to check if there is news about this feature.

@jech
Copy link
Owner

jech commented Apr 16, 2021

Sorry. I'm currently aiming at getting a stable 0.3.3 out, with no new features, just fixes. I'll think about this after 0.3.3 is out.

@jech jech force-pushed the master branch 5 times, most recently from 5c390a8 to b1bb427 Compare April 30, 2021 18:46
@jech jech force-pushed the master branch 2 times, most recently from ace55c3 to 32663db Compare May 9, 2021 15:49
@jech jech force-pushed the master branch 3 times, most recently from 6eeac02 to de78f3c Compare May 18, 2021 11:51
@jech jech force-pushed the master branch 2 times, most recently from 4f84b48 to 0d2ca28 Compare July 15, 2021 23:40
@Mejans
Copy link

Mejans commented Aug 26, 2021

I've just made this PR #103 :)

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.

Internationalisation
3 participants