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

Add a few missing strings for translation in crowdin #3252

Merged

Conversation

olorinmaia
Copy link
Contributor

@olorinmaia olorinmaia commented Dec 23, 2023

First part of adding strings to make xDrip+ more translated :)

@olorinmaia
Copy link
Contributor Author

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.

@DiBochev
Copy link

DiBochev commented Jan 3, 2024

Thank you for your work. This could be very helpful :)

@olorinmaia
Copy link
Contributor Author

@jamorham could you please review this when you got time?

@Navid200
Copy link
Collaborator

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.
I cannot tell you that this is right or wrong. But, I think it is inconsistent.

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.
You see that a string already exists for minutes. And you use it.
But, later, someone needs to change minutes to minutes: as an example. Then, what will happen if the same string has been used everywhere for it? I don't know the strategy in that respect.

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.

@olorinmaia
Copy link
Contributor Author

olorinmaia commented Jan 17, 2024

Thanks for your review @Navid200

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. I cannot tell you that this is right or wrong. But, I think it is inconsistent.

I agree, will adjust so those strings are consistent with what is done other places.

Like this?

    <string name="audio_focus_dont_adjust_other_app_sounds">Don\'t adjust other app sounds</string>
    <string name="audio_focus_lower_volume_of_other_apps">Lower volume of other apps</string>
    <string name="audio_focus_pause_other_apps_playing_audio">Pause other apps playing audio</string>
    <string name="audio_focus_pause_all_other_sounds">Pause all other sounds</string>

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.

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.

Translation is quite tricky. You see that a string already exists for minutes. And you use it. But, later, someone needs to change minutes to minutes: as an example. Then, what will happen if the same string has been used everywhere for it? I don't know the strategy in that respect.

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.

image

If I was to review this, one thing I would have to verify would be to ensure that nothing changes for English.

I have verified this locally, and you can also see from code that original text is preserved.

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.

@olorinmaia
Copy link
Contributor Author

@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.
I started working on this while I was translating Norwegian from 60% onwards to 100%, and as more got translated more I discovered was missing strings, that's why there is so many commits in this PR.

@jamorham
Copy link
Collaborator

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.

Copy link
Collaborator

@jamorham jamorham left a 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

@jamorham jamorham merged commit 81e6578 into NightscoutFoundation:master Mar 6, 2024
@olorinmaia olorinmaia deleted the add_missing_strings branch March 6, 2024 17:57
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