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

GSoC 2024: Internationalization Implementation for Zimit Frontend #51

Closed
wants to merge 50 commits into from

Conversation

Onyx2406
Copy link

@Onyx2406 Onyx2406 commented Mar 10, 2024

Fixes #46
See also openzim/overview#28

Overview

This pull request introduces comprehensive internationalization (i18n) support to the Zimit frontend.

What Has Been Done

  • Vue I18n Integration: Implemented Vue I18n for managing translations within the application, with support for lazy-loaded language files.
  • Language Selection: Added a language selection dropdown in the navbar, enabling users to switch languages dynamically. This feature is backed by automatic detection of the user's browser language, setting the application's default language accordingly.
  • English and French Translations: Currently, the application supports English and French, with key UI elements and prompts available in both languages.

What Is Left

  • TranslateWiki Integration: Formal integration with TranslateWiki is pending.
  • Automated Browser Language Detection Improvement: While initial support for detecting the browser's language is implemented, further testing and refinements is needed across different browsers and user configurations.
  • Preparation for TranslateWiki: Organizing and documenting the translation file structure to facilitate future integration with TranslateWiki.
  • Expand Language Support: Adding more languages based on community demand and contributions.

Screenshots

image

@kelson42 kelson42 requested a review from benoit74 March 10, 2024 13:48
@kelson42
Copy link
Contributor

@Onyx2406 Thank you for your PR, here a few questions:

@Onyx2406
Copy link
Author

@Onyx2406 Thank you for your PR, here a few questions:

It does fix #46, but this PR only adds translations to the strings within this repository using Vue I18n. It doesn't translate the form details from Zimfarm. This PR only contains code to solve #46 and not #28. (I wanted to refer to openzim/overview#28).

@kelson42
Copy link
Contributor

@Nikerabbit @Onyx2406 Do you know if this translation file can be indgested by Translatewiki?

@Onyx2406
Copy link
Author

@kelson42, they are using a similar structure at https:/mathjax/MathJax-i18n, so it should work.

Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you; it's promising.

  • We need RTL support
  • I see that human_size_limit and human_time_limit are not i18n
  • Languages list if static at the moment: requiring manually modifying two files to add a new locale. With translatewiki adding more all the time, we'd want something automatic

FYI (but it's a different PR) I believe there might be some strings in the backend (error message?) that would also require integration to completely close this

Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good job, I'm happy to see we are already quite close to have completed this task.

I think you've forgot to i18n the footer as well.

I propose to split the integration with TranslateWiki to another PR, we would already benefit from merging this PR once concerns have been addressed (Right To Left support is very important, I don't know if it works as of today, please test and confirm/adapt). @rgaudin are you ok with it?

Regarding @rgaudin comment about the automatic detection of new languages added, I do not think it is mandatory, especially it should not break lazy loading of translations. I do not mind to have to edit one more file everytime a new language is added (I imagine TranslateWiki folks might even be able to do it themselves when they add new languages).

Would you mind to create the "secret" qqq language where TranslateWiki expects to have translation instructions in English?

TranslateWiki Integration: Formal integration with TranslateWiki is pending.

I will take care of that once PR is merged

Automated Browser Language Detection Improvement: While initial support for detecting the browser's language is implemented, further testing and refinements is needed across different browsers and user configurations.

Do you have something special in mind to be tested? Or is it just "normal" testing?

Preparation for TranslateWiki: Organizing and documenting the translation file structure to facilitate future integration with TranslateWiki.

I think the only thing left is to add the qqq magic language with translation instructions. Do you have something else in mind?

FYI (but it's a different PR) I believe there might be some strings in the backend (error message?) that would also require integration to completely close this

Definitely a second step, let's finish this first one for now.

ui/src/locales/en.json Outdated Show resolved Hide resolved
ui/src/locales/en.json Outdated Show resolved Hide resolved
ui/src/locales/en.json Show resolved Hide resolved
@Nikerabbit
Copy link

@Nikerabbit @Onyx2406 Do you know if this translation file can be indgested by Translatewiki?

Yes, we do support nested JSON. Also FYI we have recently updated our guidance for new projects: https://translatewiki.net/wiki/Translating:New_project

@Onyx2406 Onyx2406 requested a review from benoit74 March 11, 2024 17:28
@Onyx2406
Copy link
Author

Onyx2406 commented Mar 11, 2024

@benoit74 Regarding splitting the integration with TranslateWiki into another PR, I completely agree. I have added the footer i18n.

I'll ensure that RTL support is tested and functional before merging. However, as the application currently supports only two languages which don't require RTL support, We can add RTL support to a future PR when additional RTL languages are added?

I've added the qqq language file for translation instructions.

For the automated browser language detection improvement, I'll do further testing across different browsers, so it's a normal testing and nothing special.

@benoit74
Copy link
Collaborator

Thank you ! Regarding RTL support, I do not mind to split it into another PR if there is too much work. It is however a pre-requisite for TranslateWiki integration and mostly a requirement for us, we have to translate the website into Farsi / Persian and already have persons ready to do the translation, so it will be in use quite quickly after TranslateWiki integration. Doesn't need to be perfect, but at least it should look pretty (e.g. I don't think we necessarily have to transfer all left buttons to the right and vice-versa, but at least the text should be readable and preferably right aligned when it was left aligned).

@Onyx2406
Copy link
Author

Alright. In that case, it's better to push commits for RTL Support in this PR itself.

@benoit74 benoit74 self-requested a review April 8, 2024 06:13
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, we are getting closer but you broke few things in your last commits (see conversations)

Please do not spend too much time fine-tuning the exact words of French and Farsi translations, since you've made them with an automated translation tool we will not merge them anyway, they will have to be translated by native speakers on TranslateWiki.

Other topics to address:

  • you updated all locales but not the qqq.json, while it is probably the most important one
  • why did you sometimes place HTML code inside the text to translate (e.g. app.thanksToMozilla) and other times you split the text is many substrings (e.g faq.gotErrorDesc1 and faq.gotErrorDesc2) ? I believe it would be way simpler in most situations (e.g. not when HTML code is too complex, but in every other situations) to always have HTML code inside the translation and one single string to translate, it will make the translator work way easier rather than being presented with only few words out of any context, or even a dot or a comma to not forget at the beginning of a translation
    • e.g you should have only one footer string (app is a bit vague) and its value should be Powered by <a target="_blank" href="https://kiwix.org">Kiwix</a> and <a target="_blank" href="https://webrecorder.net">Webrecorder</a>, thanks to a <a target="_blank" href="https://www.mozilla.org/moss/">Mozilla Open-Source Support</a> Award
  • limits in Request.vue JS code are still not handled properly:
    • they are not translated like in the Faq.vue
    • pluralization is not handled
  • "+" buttons of the FAQ are still not on the left when RTL language is used
    • please have a look, if it is too complex I do not mind to move this to an issue for later fix, it is not a big usability concern

ui/src/locales/en.json Outdated Show resolved Hide resolved
ui/src/locales/en.json Outdated Show resolved Hide resolved
ui/src/locales/fa.json Outdated Show resolved Hide resolved
… within translation strings for better clarity and ease of translation. Properly handled limits and implemented pluralization in Request.vue. Fixed missing localized strings in Request.vue and NewRequest.vue
@Onyx2406 Onyx2406 requested a review from benoit74 April 8, 2024 19:04
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, we are getting closer to merging this.

Still some comments in conversation plus few below.

  • I just realized that the zimit logo was previously centered on the page while it is not left or right ; you must put it back to center (sorry to not have realized this before, I did not got it right)
  • FYI, we will not integrate with TranslateWiki right away, I will open an issue with details on what still has to be done (but does not have to be addressed in this issue)

ui/src/components/FaqEntry.vue Outdated Show resolved Hide resolved
ui/src/components/NewRequest.vue Outdated Show resolved Hide resolved
@benoit74
Copy link
Collaborator

benoit74 commented Apr 9, 2024

New issue about what is left (not part of this issue / PR): #53

@Onyx2406 Onyx2406 requested a review from benoit74 April 9, 2024 17:07
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, looks like we didn't understood us well regarding the centering of the logo. I wanted to center only the logo, but not move the language button.

Expected UI in English:

image

Expected UI in Farsi:

image

Rest is OK now

@Onyx2406 Onyx2406 requested a review from benoit74 April 11, 2024 08:10
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not working as expected, logo size is altered.

Wouldn't it be simpler to move the select box out of the header, keep the header as it was and put an absolute positioning to the select box?

ui/src/components/NavBar.vue Outdated Show resolved Hide resolved
ui/src/components/NavBar.vue Outdated Show resolved Hide resolved
@Onyx2406 Onyx2406 requested a review from benoit74 April 11, 2024 09:53
@Onyx2406
Copy link
Author

Wouldn't it be simpler to move the select box out of the header, keep the header as it was and put an absolute positioning to the select box?

This worked, thanks!

Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing ... Are you constantly generating code with a code generator? This is pretty annoying to spend so much time in reviews detecting code which means just nothing, or changes which are meaningless (random spaces added everywhere, ...)

ui/src/components/NavBar.vue Outdated Show resolved Hide resolved
@Onyx2406 Onyx2406 requested a review from benoit74 April 13, 2024 11:47
@benoit74
Copy link
Collaborator

Thank you, unfortunately you again modified the indentation of many files.

I'm a bit sorry about that, but I will take over the end of this PR from this point, because I need to activate prettier and eslint on this, and this is way beyond the scope of this PR.

Please consider that work is done from this point, I will finish what needs to be done. I will convert this to draft so that it is not merged by mistake.

Thank you.

@benoit74 benoit74 marked this pull request as draft April 15, 2024 12:18
@benoit74
Copy link
Collaborator

Done on our own finally

@benoit74 benoit74 closed this Oct 10, 2024
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

Successfully merging this pull request may close these issues.

Interface should be multilingual
5 participants