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

i18n - Support i18n #4

Closed
wants to merge 1 commit into from

Conversation

icrc-jofrancisco
Copy link
Contributor

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.

Summary

This PR I use the ngx-translate module to translate labels and other sections that are not covered in the concepts.

Screenshots

None.

Issue

None.

Other

None.

@gracepotma
Copy link

@ibacher or @hadijahkyampeire might either of you be able to review this PR? Blocker for ICRC.

Copy link
Contributor

@hadijahkyampeire hadijahkyampeire left a comment

Choose a reason for hiding this comment

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

Except for the conflicts, this looks great, thanks @icrc-jofrancisco

@gracepotma
Copy link

Thank you @hadijahkyampeire for your eyes on this :) I briefly discussed this w/ @ibacher today after I wrote my comment above - he recommended that you take ownership for reviewing this PR and getting it to a merge-ready state. He did have a word of caution - the size of this is large, and he mentioned it needs additional work, because it currently hardcodes some translations as a proof of concept - but really, this needs work to be extensible and run successfully.

How does that sound to you @hadijahkyampeire ?

@hadijahkyampeire
Copy link
Contributor

Thanks @gracepotma I will definitely take a closer look today and see what needs to be added. I am not so familiar with Angular but I will try my best 👍

@hadijahkyampeire
Copy link
Contributor

@icrc-jofrancisco @ibacher I realized that the first form for details is automatically translated to French when I run this branch, is that the expected behavior? cc @gracepotma

@icrc-jofrancisco
Copy link
Contributor Author

To test this functionality, it is necessary to have a particular environment.
To ensure that the translation files are loaded into the database, you will have to run this development on the initz.
After that, thispatient chart development will load this development of formentry.

@hadijahkyampeire
Copy link
Contributor

To test this functionality, it is necessary to have a particular environment. To ensure that the translation files are loaded into the database, you will have to run this development on the initz. After that, thispatient chart development will load this development of formentry.

@ibacher does the above comment mean that this work depends on the other PR being merged so that we get translations flowing?

@gracepotma
Copy link

@ibacher just re-escalating this to you ;) LMK if you want to debrief this.

@ibacher ibacher closed this Feb 13, 2023
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.

4 participants