This seems to be a small leftover from development, please remove it.

Please use i18n for this as well.

Same for all expand_text props.

Please investigate if i18n supports JSX so we can put formatted text there.

Sure thing I'll implement all the changes, did a bit of research and found that i18n doesn't support JSX.

Is there a library in npm that would add support?

Does this needs to be split into three separate phrases?

Does this needs to be split into three separate phrases?

No, I'll change it.

Does this needs to be split into three separate phrases?

No, I'll change it.

Thanks, please also make sure that other similar "splits" (if any) are fixed.

Does this needs to be split into three separate phrases?

No, I'll change it.

Thanks, please also make sure that other similar "splits" (if any) are fixed.

Yes will make sure

Is there a library in npm that would add support?

I looked around and didn't find any library which supports JSX.

I just realized that we can put JSX into the Manual.js itself and use i18n for the strings only, formatting will be probably the same for every language. So no problem here I guess...

I just realized that we can put JSX into the Manual.js itself and use i18n for the strings only, formatting will be probably the same for every language. So no problem here I guess...

Yes I guess so too,
I have implemented all the changes please have a look.

Could you please move the "language" button closer to "settings" button.

Also I think it would be nicer to change the label from "Select language" to currently selected language, for English it will be "en" (see ISO 639-1).

Any comments @jskladan?

OKay great, I'll implement the changes

The label is now showing the selected language, @lbrabec please have a look!

@manishakanyal After looking at the PR for quite some time, I must say I'm not the biggest fan of how the code is prepared for the translations.

Even putting aside the fact, that preparing phrases for translations before all the texts are finished is pretty premature, I don't like the way the phrases are split up quite a lot. Take, for example, this:

-                       Have a look at the matrix. The rows are the testcases, and the columns are
-                       the environments. Most of the time, these are {environment_examples} for the{" "}
-                       {testtype} testcases, but you can also encouter a generic <code>Result</code>{" "}
-                       column, which usually means you can use any media/architecture available.
+                       {i18n.t("phrases.Have_a_look")} {environment_examples}{" "}
+                       {i18n.t("words.for_the")} {testtype}{" "}
+                       {i18n.t("phrases.testcases_but_you_can")}{" "}
+                       <code>{i18n.t("words.Result")}</code>{" "}
+                       {i18n.t("phrases.Column_which_usually")}

as an example. Changing this in the future will be an absolute PITA, and the way the text is split up does not really make much (if any) sense for languages with different sentence structure than english.

Sadly, preparing code to be translated can rarely be done by an automated tool - which I'm almost certain u used, based bits like these throughout the code:

-             Make sure your system is up-to-date: <code>sudo dnf update --refresh</code>
+             {i18n.t("phrases.make_sure_your")}
+             <code>{i18n.t("phrases.sudo_dnf_update_refresh")}</code>

I guess most of this can be attributed to the automated tool and the fact that we need to deal with JSX, as you have briefly touched on here:

Is there a library in npm that would add support [for JSX]?

I looked around and didn't find any library which supports JSX.

I understand that google searches are quite personalized, but nevertheless, this: https://react.i18next.com/ is what I had as a second link on the first page of a Goole search for "jsx i18n" [https://www.google.com/search?q=jsx+i18n]. It seems to be solving all the issues here - e.g. https://react.i18next.com/guides/the-drawbacks-of-other-i18n-solutions#can-you-translate-combined-jsx-nodes-in-one-sentence

Shold we go forward with preparing the code for translations now, I'd much prefer you used this. The resulting code is much more readable, and the translations of (what will be) a very tag-heavy strings will make much more sense at least on a sentece-level (if not a paragraph-level) availble with the i18next library.

@lbrabec tagged for notice ^^

@jskladan, @lbrabec, I have removed splits, please have a look.

This should be <0></0>, see https://react.i18next.com/latest/trans-component#how-to-get-the-correct-translation-string

I don't think that tag attributes (target=, rel=, href= etc..) belong in translation. Also the tag <1> is closed with </a> instead of </1>

<code></code> should be replaced with "number tag" as others.

I believe that we shouldn't translate words such as "beta" and "final", they are technical terms.

Reducers should be pure functions, settings an item in local storage is a side effect thus making the reducer impure. Please move it to action creator.

Also, I don't see any i18-next initialization (such as this https://react.i18next.com/getting-started#basic-sample)

It seems like you're not using i18next at all, and still using i18n-js

You should use useTranslation() hook instead (https://react.i18next.com/latest/usetranslation-hook)

@jskladan, @lbrabec I have implemented the changes please have a look

@jskladan, @lbrabec I have implemented the changes please have a look

This string seems to be wrong. If you change the key to something non existent, there will be debug output in console with correct string, which is:
We already mentioned Bodhi, several times. Instead of using the <2>fedora-easy-karma</2> interface for submitting Karma and/or learning about the Update, you can visit Bodhi directly.<br/><5>fedora-easy-karma</5> shows the URL of the relevant update near the bottom of the text output - look for URL like <8>https://bodhi.fedoraproject.org/updates/FEDORA-2019-00870e8bfc</8>.<br/>You can have a look at other people's comments, the afore-mentioned Bugs or Test Cases, and even submit the Karma directly. Just use your FAS credentials to log-in.

Please go through all the strings and make sure they are correct, incorrect string can cause missing words.

I also noticed these warnings in console:

i18next::translator: missingKey undefined translation None_of_the_above_looks_interesting_to_you None_of_the_above_looks_interesting_to_you
i18next.js:22 i18next::translator: missingKey undefined translation source source
i18next.js:22 i18next::translator: missingKey undefined translation .on_Pagure .on_Pagure
i18next.js:22 i18next::translator: missingKey undefined translation version version
i18next::translator: missingKey en translation .on_Pagure .on_Pagure

This also doesn't look right:

i18next.js:22 i18next::translator: key "None_of_the_above_looks_interesting_to_you" for languages "en" won't get resolved as namespace "translation" was not yet loaded This means something IS WRONG in your setup. You access the t function before i18next.init / i18next.loadNamespace / i18next.changeLanguage was done. Wait for the callback or Promise to resolve before accessing it!!!

@jskladan, @lbrabec I have implemented the changes and removed warnings please have a look

Thanks for removing the warnings. However We_already_mentioned_bodhi still renders wrong, because <code>fedora-easy-karma</code> has wrong "number tag" in translation string.

@lbrabec, I have corrected the number tag, please have a look.

In FedoraManualTesting.js there is double "or" in:

case "Base":
      environment_examples = (
        <span>
          <code>Workstation</code>, <code>Server</code>, <code>{t("or")}</code> or{" "}
          <code>Minimal</code>
        </span>
      )
      break

Please remove the one in <code> a use translation for the plain text one.

Curly braces are not rendered properly, the tooltip shows {{result|pass|YOUR_NAME_HERE|}}, same for key Once_you_get_the_bug

Any comments @jskladan ?

After a quick look at the changes, I can only point out that the translation.json file is polluted with dead strings never used anywhere in the code. Please make sure the file only contains what is necessary, so I can have a proper look at how it's structured now.

Thanks.

@jskladan, @lbrabec I have implemented the changes and removed dead strings please have a look

@manishakanyal I'm not sure if I was not clear enough, but looking at the code, there is still a lot of strings in the translation.json file, that are not used at all.

Picking just randomly (know that these are absolutely not all of them):
* https://pagure.io/fedora-qa/landingpage/pull-request/84#_4__3 (yes, it is the first line of the translation.json file)
* https://pagure.io/fedora-qa/landingpage/pull-request/84#_4__21
* https://pagure.io/fedora-qa/landingpage/pull-request/84#_4__52
* https://pagure.io/fedora-qa/landingpage/pull-request/84#_4__71

... and the list goes on.

Please, make sure all the unused strings in the translation.json file are not present. I understand that this might seem tedious, but right now, the file is next to unuseable, and the quality of the prep-work for the translation can not really be judged seriously without me spending insurmountable amount of time on it.

Let this be a lesson for when you next use an automated tool to do something instead of you - automated tools have their uses, but the base on which the tools work must be precise, and the results of the tools must be scrutinized, not just blindly used.

@jskladan I don't know how I missed those. Probably because the file is huge.
Thanks for pointing it out. I've removed the unused strings and pushed the code again.

Also, I want to mention the fact that I didn't use any script to generate the strings but did on my own.

The left unused strings that you are misunderstanding as script generated are because I had implemented translation using i18n and not react-i18n. There were strings for every split which we later removed by using Trans of react-i18n.
The split's strings were left by mistake.

Also, I want to mention the fact that I didn't use any script to generate the strings but did on my own.

OK, good to know. Since you created the translations manually, could you explain the benefits of having the expand_class attribute translatable here: https://pagure.io/fedora-qa/landingpage/pull-request/84#_21__428 or here: https://pagure.io/fedora-qa/landingpage/pull-request/84#_21__450 ?

These are, of course, just a couple examples I randomly found just scrolling through the code. It looked like an autogeneration-caused error, but I guess I'm missing something important here, since this was (by your own words) a concious choice on your part.

OK, good to know. Since you created the translations manually, could you explain the benefits of having the expand_class attribute translatable here: https://pagure.io/fedora-qa/landingpage/pull-request/84#_21__428 or here: https://pagure.io/fedora-qa/landingpage/pull-request/84#_21__450 ?

These are, of course, just a couple of examples I randomly found just scrolling through the code. It looked like an autogeneration-caused error, but I guess I'm missing something important here since this was (by your own words) a conscious choice on your part.

@jskladan, that's totally my fault I'm sorry, as @lbrabec told me to use react-18n for the texts inside expand_text I didn't notice whenexpand_text and expand_class seem the same to me.

I'll do the changes.