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

Request to change a localization script #748

Open
ngdangtu-vn opened this issue Sep 28, 2020 · 12 comments
Open

Request to change a localization script #748

ngdangtu-vn opened this issue Sep 28, 2020 · 12 comments
Assignees

Comments

@ngdangtu-vn
Copy link

As I mentioned in here: https://poeditor.com/projects/po_edit?&id=257157&id_language=177#row-term-146048017
I would like to modify about_how_do_you_calculate_the_number_of_cigarettes_message_2 script for a better translation purpose.

Since I'm confident that I can do this. Please assign this task to me. I will submit PR in later day.

@amaury1093
Copy link
Member

Thanks @ngdangtu-vn!

Yes, in the app, the full sentence in English is:

This app was inspired by Berkeley Earth’s findings about the 
equivalence between air pollution and cigarette smoking
. The rule of thumb is simple: one cigarette per day (24h) is the rough equivalent of a PM2.5 level of 22

And the 2nd line is a link to an external website. That's why I separated this one sentence into 3 parts:

  • about_how_do_you_calculate_the_number_of_cigarettes_message_1
  • about_how_do_you_calculate_the_number_of_cigarettes_link_1
  • about_how_do_you_calculate_the_number_of_cigarettes_message_2

If you can find a better way to separate this sentence, while still keeping the external link, please go ahead with a PR!

@ngdangtu-vn
Copy link
Author

@amaurymartiny
Could I have permission to replace expo-localization with react-i18next? I think it can solve our current problem.

@amaury1093
Copy link
Member

amaury1093 commented Oct 3, 2020

It's in my mind to move away from expo-localization (i18n-js), i saw some limitations, so okay to move away 👍 .

How does react-i18next compare to react-intl?

@ngdangtu-vn
Copy link
Author

How does react-i18next compare to react-intl?

If it's possible, please give me 2 days, I will do some researches about it. :)

@amaury1093
Copy link
Member

Thanks! 🙏

@ngdangtu-vn
Copy link
Author

One thing that is clearly difference between react-i18next and react-intl is Components:

  • react-intl has rich components style, up to 13 one. Thanks to that, it will be merged to the React JSX style very well. It could be a good choice for clean. Moreover, one the translator side, scripts are more readable as well.
  • react-i18next is on the other hand, it favour the functional style. We can said it is pretty much the expo-localization with
    Trans extensions. Trans extension, the selling point, is a component that works pretty much like react-intl. So we will have a mix style syntax in our code. The translated scripts are somewhat vague (if we use Trans a lot), check this example out: {"the_key": "Welcome to <1>React and react-i18next</1>"}. Its selling point is a well document and many guides, or at least that is my point of view. Some can be very helpful.

My conclusion is if your app has more static script (require less format, not have many words, like 'next', 'back', '404 error') then react-i18next is the one. Otherwise, a good coherent syntax for a complex translation, react-intl is the choice.

(Sorry for being late, it's internet fault)

@amaury1093
Copy link
Member

Thanks a lot for this overview!

My conclusion is if your app has more static script (require less format, not have many words, like 'next', 'back', '404 error') then react-i18next is the one.

I overall really prefer a simple t('...') for translating (instead of components everywhere), and I think >90% of strings in the App are static. We can use the Trans component in e.g. the case you described in your first post. So let's use react-i18next! 🚀

Would you like to create a PR for this?

@ngdangtu-vn
Copy link
Author

Yes, I will try to implement it and create PR.

@ngdangtu-vn
Copy link
Author

@amaurymartiny
I have successfully implement the i18next for main part (I haven't deal with language picker and language detector yet, but it shouldn't be to much problem). Now, I would like to discuss with you about improve script key to take advantage of i18next. It means, we will move from:

// old script
{"about_box_per_day": "per day" }

To this:

// new script
{
   "about": { "box_per_day": "per day"} // "about" is namespace
   // And it can even be more complex with Trans
}

This means we have to upgrade the translation scripts. And I would like your opinion about how should we organize the scripts. My solution is we separate it on namespace level with 'home', 'about', 'warning', 'error', 'glossary'. Also, should I pull request right now so you could see an overview of how new translation tool works?

@amaury1093
Copy link
Member

Thanks! Yes, organizing the scripts with namespaces is a good idea.

Is it possible to make nested namespaces? If yes, I believe the easiest way to organize namespaces is to follow the folder structure already in place:

{
  "components": {
    "currentLocation": {
      // scripts inside the CurrentLocation component
    },
  },
  "screens": {
    "about": {
      // scripts inside the About screen
    },
    "home": {
      // scripts inside the Home screen
    }
  }
}

This seems like the most obvious way to me. Maybe you have a better idea?

BTW, no worries to break the current translation files script, we can also add a small script in a future PR to convert from the old to the new.

Also, should I pull request right now so you could see an overview of how new translation tool works?

Yes, please!

@ngdangtu-vn
Copy link
Author

ngdangtu-vn commented Oct 10, 2020

Is it possible to make nested namespaces
I don't think we have nested namespaces, nesting is another story to tell (quite powerful and a bit complex). In your suggestion code, it is still queriable like this:

{
  "screens": {
    "about": {
      "x": "abc"
    }
}
const { t } = useTranslation('screens')

t('about.x') // abc

While this is good for components namespace. Using this on screens ns would disable one advantage of its, faster loading. When this useTranslation('screens') was called, the tool will make sure only summon screens ns therefore it will be faster. If the screens is used as current dir structure, that purpose will be lost. So may I suggest to use this instead:

{
  "home_screens": {
    "x": "abc"
  },
  "about_screens": {
    "y": "def"
  }
}

We might have more namespace as the app grows up, but it will have better performance.

@amaury1093
Copy link
Member

I wasn't aware of the performance hit. The way you described sounds good 👍

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

No branches or pull requests

2 participants