e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

🔖 Re-design of Modes/Purpose Multilabel Translations #1007

Open sebastianbarry opened 1 year ago

sebastianbarry commented 1 year ago

What's the issue with the current modes/purpose multilabel translations?

  1. The translations for the modes/purposes are in either the default or dynamic label-options.json file. If we move the translations to the international translation files es.json or en.json, we won't need to have these translations in the label-options files requiring textual changes in the future to consider this "easily forgotten" file?
  2. (per @shankari in https://github.com/e-mission/e-mission-phone/pull/1055#issuecomment-1755769682) There are other places where we want to unify the handling of modes (notably the public dashboard calculations being tracked in https://github.com/e-mission/em-public-dashboard/issues/89) and we will be revisiting this when we include energy calculations in the app dashboard

Regarding (1), I put my thoughts on how this implementation could work, along with some pros/cons in https://github.com/e-mission/e-mission-phone/pull/1055#issuecomment-1755741714. The issue with my proposed implementation is that some programs want to change the modes often, meaning that because "Every time a new mode/purpose is added for any given program, the phone code will need to be updated to reflect that because "en.json" is by default located within the e-mission-phone code" Which is not realistic to update the phone code for the frequency of changes that programs such as Lao require

JGreenlee commented 1 year ago

Ok, so if label_options is defined in the dynamic config, it will fetch that file, and the translations will need to be inside that file. If there is no label_options, it will use the default label-options.json.sample. This file will not have translations in it, so we will use translations from en.json / es.json / lo.json.

It should not be hard to support both of these usage patterns using the same mechanism. The labelOptions might have translations or it might not. As we go through each label, we can check if there's a translation for it in the same file, if so we'll use it - otherwise, we'll fallback to looking in en.json / es.json / lo.json.

sebastianbarry commented 1 year ago

By default, we should check the dynamic label-options file, that way programs can use the "wordage" they prefer. For example, the en.json file might say "rickshaw", but the Lao translation might be more accurate to use "tuk tuk" which is more region-specific

sebastianbarry commented 1 year ago

I am finding myself stuck on using useTranslate.t from react-i18next. If I try to use

const { t } = useTranslation();

in confirmHelper.ts's getLabelOptions() function, the function ends without error.

Before the line gets executed Screenshot 2023-10-12 at 11 56 52 AM
After the line gets executed, new error comes up Unhandled Promise Rejection: Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons: 1. You might have mismatching versions of React and t... Screenshot 2023-10-12 at 11 57 34 AM
App working fine without line const { t } = useTranslation(); Screenshot 2023-10-12 at 11 45 55 AM
App not working with line const { t } = useTranslation(); Screenshot 2023-10-12 at 11 52 16 AM

@JGreenlee do you have any advice for this?

JGreenlee commented 1 year ago

Like it says, you can't use a hook there. You can only use hooks in React code: at the top of a component, or at the top of another hook.

To perform translation in non-React code:


import i18next from "i18next";
...
...
const value = i18next.t('key');
sebastianbarry commented 1 year ago

Thanks @JGreenlee , that worked!

Closed my old PR and created a new one with the changes described in this issue: PR https://github.com/e-mission/e-mission-phone/pull/1065

sebastianbarry commented 1 year ago

The only other thing I could think to add to this issue, is to go through the other language files in e-mission-translate and add default translations for multilabel options

sebastianbarry commented 1 year ago

Created multilabel translations for en.json and lo.json: https://github.com/e-mission/e-mission-translate/pull/47