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

128 - Translations and messages improvements #234

Merged
merged 12 commits into from
Dec 2, 2021
Merged

Conversation

mrsarm
Copy link
Contributor

@mrsarm mrsarm commented Nov 25, 2021

  • Add missed translations for some buttons and messages, and fixed others.
  • Add French and Spanish translations.

Issue: #128

@mrsarm
Copy link
Contributor Author

mrsarm commented Nov 25, 2021

Hey @abbyad , would you mind to review?

Some of the changes you suggested were applied before, and I have fixed others here. I also added more French translations, and Spanish because although we don't use them, it's easy for me.

@mrsarm mrsarm marked this pull request as ready for review November 25, 2021 22:38
@mrsarm mrsarm force-pushed the fix-missed-button-trans branch from 47d25de to 6092f78 Compare December 1, 2021 18:05
Copy link
Contributor

@abbyad abbyad left a comment

Choose a reason for hiding this comment

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

Looks good! Made some minor edits, and also removed CHT-Core throughout since it isn't meant to be used as the branded tool. We should stick to "CHT server" or "CHT instance" as needed in the context.

On a related note, do we even need to say CHT, or would just something like "the server is not ready"? Keeping it generic might help if these messages are coming up on a rebranded app (eg partner branded instead of CHT branded).

src/main/res/values-fr/strings.xml Outdated Show resolved Hide resolved
src/main/res/values-fr/strings.xml Outdated Show resolved Hide resolved
src/main/res/values/strings.xml Outdated Show resolved Hide resolved
src/main/res/values/strings.xml Outdated Show resolved Hide resolved
src/main/res/values/strings.xml Outdated Show resolved Hide resolved
src/main/res/values-fr/strings.xml Outdated Show resolved Hide resolved
src/main/res/values-es/strings.xml Outdated Show resolved Hide resolved
@mrsarm
Copy link
Contributor Author

mrsarm commented Dec 2, 2021

Keeping it generic might help if these messages are coming up on a rebranded app (eg partner branded instead of CHT branded)

Totally agree, I have applied all the changes suggested, and checked there are no remaining CHT-Core references. Please take a look again.

@mrsarm mrsarm requested a review from abbyad December 2, 2021 00:28
@mrsarm mrsarm force-pushed the fix-missed-button-trans branch from 415b5d6 to 8fb1e8c Compare December 2, 2021 13:09
@abbyad
Copy link
Contributor

abbyad commented Dec 2, 2021

Keeping it generic might help if these messages are coming up on a rebranded app (eg partner branded instead of CHT branded)

Totally agree, I have applied all the changes suggested, and checked there are no remaining CHT-Core references. Please take a look again.

It seems as though we agree that the specific mentions to the CHT in these messages should be removed, is that already part of this PR? (I still see "CHT server is not ready, please try again soon")

@mrsarm
Copy link
Contributor Author

mrsarm commented Dec 2, 2021

It seems as though we agree that the specific mentions to the CHT in these messages should be removed, is that already part of this PR? (I still see "CHT server is not ready, please try again soon")

So so, I have changed all references of CHT-Core to just CHT or "server" depending of the context, @abbyad do you suggest to also replace "CHT server" for just "Server" ?

@abbyad
Copy link
Contributor

abbyad commented Dec 2, 2021

Are these messages seen only when using the unbranded app? If so then CHT may be fine. But if a message like "CHT server is not ready, please try again soon" (errAppUrl_apiNotReady) can pop up for branded flavors, then we should avoid the use of "CHT" in that case. The other labels seem pretty specific to the unbranded flavor, so should be fine as they are already, but will let you be the judge on the others.

@mrsarm
Copy link
Contributor Author

mrsarm commented Dec 2, 2021

Yes @abbyad , all the translations that refers to CHT here are for the Unbranded flavor, and used when you try to setup the CHT instance URL, so I think we are fine, so if it is OK for you please give it the ✔️ .

Copy link
Contributor

@abbyad abbyad left a comment

Choose a reason for hiding this comment

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

Nice work!

@mrsarm mrsarm merged commit 49a3c20 into master Dec 2, 2021
@mrsarm mrsarm linked an issue Dec 2, 2021 that may be closed by this pull request
@mrsarm mrsarm deleted the fix-missed-button-trans branch December 2, 2021 20:42
jkuester pushed a commit that referenced this pull request Dec 2, 2021
Add missed translations for some buttons and messages, and fixed others.

Co-authored-by: Marc Abbyad <[email protected]>
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.

Update labels to use generic app name
2 participants