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

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

Open
sebastianbarry opened this issue Oct 10, 2023 · 7 comments
Open

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

sebastianbarry opened this issue Oct 10, 2023 · 7 comments

Comments

@sebastianbarry
Copy link

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 Deault label options e-mission-phone#1055 (comment)) There are other places where we want to unify the handling of modes (notably the public dashboard calculations being tracked in Support custom label dropdowns from the dynamic config em-public-dashboard#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 e-mission/e-mission-phone#1055 (comment). 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
Copy link

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
Copy link
Author

sebastianbarry commented Oct 10, 2023

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
Copy link
Author

sebastianbarry commented Oct 12, 2023

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
Copy link

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
Copy link
Author

sebastianbarry commented Oct 12, 2023

Thanks @JGreenlee , that worked!

Closed my old PR and created a new one with the changes described in this issue: PR e-mission/e-mission-phone#1065

@sebastianbarry
Copy link
Author

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
Copy link
Author

Created multilabel translations for en.json and lo.json: e-mission/e-mission-translate#47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants