-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add a few missing strings for translation in crowdin #3252
Add a few missing strings for translation in crowdin #3252
Conversation
…d_missing_strings # Conflicts: # app/src/main/res/values/strings.xml
Removed typo
I've built apk and tested locally that code works as intended. As Norwegian bokmål is 100% translated it's easier to notice that there are quite a few places in the code missing string for translation in crowdin. I won't add any more to this PR. But will start working on the remainders in a new PR if / when this gets merged, that will most likely add strings for missing translation everywhere in xDrip where translation is useful for end user. |
Thank you for your work. This could be very helpful :) |
@jamorham could you please review this when you got time? |
You have done something with audio focus strings that is inconsistent with respect to the rest of xDrip strings, as far as I am aware. Instead of using the English text and underscore separators to create the titles, you have used audio_focus_choise_1 and 2 and 3 etc. And you have translated some wear strings. I really am not sure what the status/future plan/strategy of the wear portion of xDrip is. Translation is quite tricky. If I was to review this, one thing I would have to verify would be to ensure that nothing changes for English. It may seem that the strings array should be easy to review. But, I am not sure it is. Sorry I have no advice. Just some thoughts. |
Thanks for your review @Navid200
I agree, will adjust so those strings are consistent with what is done other places. Like this?
I was thinking while fixing missing translation in main app maybe do it in wear also while at it. I'll remove it as it isn't necessary to fix translation of alert coming from phone app.
I think it's smart to re-use already translated strings if certain they won't change, don't need to translate same word several times. I see it unlikely that string for minutes will change as it is used several places already, and there is already string for minute etc.
I have verified this locally, and you can also see from code that original text is preserved.
|
…g string from wear.
@jamorham I can close this and make a new PR with only 1 commit and a few less changes if it's easier to review. Let me know what you think. |
The number of commits isn't an issue, we can always squash them on merge if we want to. Its just a case of confirming nothing is broken during string extraction if it was done manually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed attention everyone who has looked at this. It looks good to me
First part of adding strings to make xDrip+ more translated :)