Localized version of web directory for further implementation and test

[HarikalarKutusu]

This PR includes all additions and modifications for the WEB directory, except the last-minute change I recognized, which is fixing #498 which should be handled...

Two resource files (sentence_collector.ftl) added for "en" and "tr" locales. The "tr" locate is for test only, not fully translated except the header, footer, and titles of each page. The English resource's texts are prefixed with "&", the non-translated Turkish sentences with "%" to check the translation is working. If no such sign is given, HTML in the code is being used.
Edit: Prefixes & tr language resource will be removed in the final PR.

There are several issues to be further checked resulting in code changes, as discussed in #479

I labeled every problematic thing we discussed with TODO / FIXME which would result in some cleanup...
Edit: Comments removed for final PR.

(This also fixes issue 487)
Edit: Fix removed as per suggestion below.

I hope everything goes well...

[MichaelKohler]

Thanks a lot for all the work! I think I will have some time this weekend to review and test this. Prefixing the strings is a nice idea, definitely makes it easier to test (and removing the prefixes and the Turkish file before merging is easy).

So far I've noticed a few things:

• Could you rebase on top of latest main branch commit? The conflicts are mostly around stuff that I added in the main branch and got added in your branch as well. If you need any help with any of the conflicts, happy to help!
• It seems that some file modes got changed on several files. Would be great if we could keep them on 644. This change unfortunately could for example happen if you're using Windows. https://stackoverflow.com/questions/1580596/how-do-i-make-git-ignore-file-mode-chmod-changes

Once I start reviewing this in detail there might be quite a few comments. I hope that won't be discouraging. If at any point you would like help to get this over the finish line, please tell me and I can help out.

Thanks again, this work is greatly appreciated.

[HarikalarKutusu]

Sorry for all the trouble. I'm not good in Git/Github, I'm used to working on Windows. Actually, with one wrong command, I managed to erase all the changes I made, this is why I'm late.

I worked on VirtualBox/Ubuntu but I had a backup on Windows side and had to replace the files from it, this must be the reason for the change in the access rights. That's easy to correct.

Rebasing is what I'm afraid of. That's how I messed up. I'll learn and try thou. How can I amend a PR, huh?

I hope that won't be discouraging.

Nope, never. This is how you learn...

[MichaelKohler]

this is why I'm late

There is no such thing as "late" when there is no deadline ;) .. and most certainly not as a volunteer contributing to an Open Source project.

Rebasing is what I'm afraid of. That's how I messed up. I'll learn and try thou. How can I amend a PR, huh?

Rebasing is one of the trickier concepts of git, indeed. Honestly, even after years of extensively using git, I'm still sometimes afraid of rebasing. Are you usually using the terminal or another app for Git operations?

If you use the terminal, here's a quick guide:

# Add a reference to this repo here
git remote add upstream git@github.com:common-voice/sentence-collector.git

# Create a "backup"
git checkout feature/localization
git checkout -b feature/localization-backup # creates a new branch based on top of your localization branch
git checkout main

# At this point you have a new branch which you could restore if something goes wrong

# Get latest changes from this repo's main branch
git pull upstream main

# Now your local main branch should have the same state as the one in this repository here

# Checkout your branch and rebase
git checkout feature/localization
git rebase main

# git will eventually shout at you that there are conflicts, these now need to be resolved

# once all the conflicts are resolved, you can run
git add web
git rebase --continue

# Now your branch should have all the commits from main, plus your commit on top of it
# Now we can push it online. The -f is needed here because you now have a completely different structure.
git push -f origin feature/localization

# In case anything went wrong, you can do the following:
git checkout feature/localization-backup # checkout the backup
git branch -D feature/localization # delete the old localization branch
git checkout -b feature/localization # create the localization branch from the backup branch
git branch -D feature/localization-backup # delete the backup branch

[HarikalarKutusu]

Thank you, that helps a lot. A simple thing like backup branch never came to my mind.

I've been using SmartGit on Windows for Git on my local server. Now I installed it on Ubuntu for its Conflict Solver mainly...

Let's see how it goes...

[HarikalarKutusu]

I finally could push it... In the meantime, I handled the Prettier and Lint errors. Web tests fail because of the following:

Error: Uncaught [Error: useLocalization was used without wrapping it in a < LocalizationProvider />.]

There are several FIXME comments related to these in the code. If you can direct me I can also solve these...

It was a bit messy with multiple commits but my heart is not strong enough to Squash / Merge them :/

[MichaelKohler]

First of all, this is looking really good! As mentioned before, I was expecting to write quite a few comments while reviewing this, and that turned out true. I kinda feel bad for dumping 61 comments on you at once, but two things for that:

• Most comments are smaller fixes needed, such as changing the ID of a string used, or removing comments we don't need anymore.
• If you need any help or clarification, please don't hesitate to reach out to me either by writing a response to a comment, or also on Matrix.

Additionally to the parts I commented in the review, there are some general topics to take care of:

• Please remove all the // LOCALIZATION VERSION comments from all the files. I sometimes commented on that, but not everywhere where it needs to be removed.
• Please fix the file modes as commented on further above in this comment thread.
• I had a quick look at the fallback mechanism. My previous statement was wrong, it seems that it already takes English as fallback language if a string is not yet translated. As you noted otherwise previously, I might have missed a use case though. Can you remove the placeholders and retest that? At this point I'm confident that we can remove all the placeholders and solely use the FTL files.

As last steps in this PR, we will then need to:

• Remove the Turkish translation file and the addition of tr to l10n.tsx
• Remove all the prefixes from the EN ftl file

Thanks again for working on this, I really can't mention that enough. Once this PR is merged we will have taken a huge step towards enabling localization for the full page. 🎉 🎉

[MichaelKohler]

This line is good, but please make sure to reset the file mode to 644.

[HarikalarKutusu]

Done for every file/dir - see my Q below.

[HarikalarKutusu]

Some quick comments before starting fixes. Others I'll handle through "resolve conversation"s...

First of all, this is looking really good! As mentioned before, I was expecting to write quite a few comments while reviewing this, and that turned out true. I kinda feel bad for dumping 61 comments on you at once, but two things for that:

No problem.

• Please remove all the // LOCALIZATION VERSION comments from all the files. I sometimes commented on that, but not everywhere where it needs to be removed.

I did that to: (1) Make them dirty so tat they are in the commit (2) It helped during conflict/resolve case. I'll remove them.

• Please fix the file modes as commented on further above in this comment thread.
  I though I did! Executed the mentioned command at least. I'll chmod them once again...

• I had a quick look at the fallback mechanism. My previous statement was wrong, it seems that it already takes English as fallback language if a string is not yet translated. As you noted otherwise previously, I might have missed a use case though. Can you remove the placeholders and retest that? At this point I'm confident that we can remove all the placeholders and solely use the FTL files.

OK, I'll test it...

• Remove the Turkish translation file and the addition of tr to l10n.tsx

OK.

• Remove all the prefixes from the EN ftl file

OK. You mean "&", not "sc_"?

a huge step towards enabling localization for the full page. 🎉 🎉

I worked on many l10n, i18n projects for 20+ years, but mostly solely, not on Git/Github... I felt it is a must for Common Voice which is multi-lingual in nature...

[MichaelKohler]

OK. You mean "&", not "sc_"?

Yes, just the &. sc- is fine for now.

[HarikalarKutusu]

Please fix the file modes as commented on further above in this comment thread.

I scanned every dir/file and corrected some. But I used before the following:

git config core.fileMode false

I'd guess that the first commit which did introduce the rights problem will not be corrected if I keep it. Right?

[MichaelKohler]

With that config set, I think you can use the following to apply the fix then.

git update-index --chmod=-x path/to/file

[HarikalarKutusu]

Plural method implementation for this one is tricky:

sc-stats-summary = The Common Voice Sentence Collector has collected
    { $sentenceCount } sentences in { $languageCount } languages!

Will it be nested? Very cumbersome for translators/Pontoon...

[MichaelKohler]

Will it be nested? Very cumbersome for translators/Pontoon...

Good point, I think for this one no pluralization is fine, we're not gonna get down to 0 or 1 anyway except for local dev, and for dev it's totally fine.

[HarikalarKutusu]

During testing I fixed several issues with my-sentences-list.tsx where I didn't put a tag inside < Localized >

[HarikalarKutusu]

Another quirk I found on the statistics page, after many interactive tests:

But when I click the "Review now!" link I get:

Is this something I introduced or just a local database issue?

[HarikalarKutusu]

Finished all comments/mistakes. I left some of them un-resolved where it should be further tested for plurals (and one for chmod).

I'll commit the changes, there might be a second pass needed.

[HarikalarKutusu]

I had a quick look at the fallback mechanism. My previous statement was wrong, it seems that it already takes English as fallback language if a string is not yet translated. As you noted otherwise previously, I might have missed a use case though. Can you remove the placeholders and retest that? At this point I'm confident that we can remove all the placeholders and solely use the FTL files.

This seems not directly applicable as of now. I think we need to move the tags to elems for this to work. A huge second pass is needed for this. As our Turkish campaign is starting tomorrow, I may not have to do it immediately...

I'd say, let's resolve this one and do another PR for optimization.

[MichaelKohler]

Thanks for all the changes. I will have a look tomorrow. I agree on the placeholders to take care of this in a subsequent PR.

One thing for now: it looks like you deleted quite a few files. Deleting a file does not reset them to the original state but actually suggests to delete them as part of the PR.

I think at this point it's just a few things, which I'd be happy to take over if you want to focus on the campaign. I have some time tomorrow. The vast majority of work is done here, I still need to review the changes, but as I said, happy to take over if needed.

[HarikalarKutusu]

One thing for now: it looks like you deleted quite a few files. Deleting a file does not reset them to the original state but actually suggests to delete them as part of the PR.

Unfortunately, I recognized that after I deleted them from GitHub PR. I thought I'll be deleting them from the commit. Now searching for ways to undo the commit from GitHub...

I still need to review the changes, but as I said, happy to take over if needed.

During the commit, VirtualBox just hanged and left it in an unstable state. I need to re-check everything now and commit afterwards.

For the placeholders: I'll do one page (Home) and leave it there.

After the commit, please take over, after 10 hours of dealing with it you just start to make mistakes...

[MichaelKohler]

Absolutely understandable, I also don't want to pressure you. I'm happy to leave this open if you want to pick it up again later on, but I'm happy to merge this ASAP and I take care of the minor things to do that. While I haven't done a second review after your changes, I really think there is not much left to do here.

[MichaelKohler]

Superseded by #507