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

Fix language changing issues #292

Merged
merged 2 commits into from
May 26, 2021
Merged

Fix language changing issues #292

merged 2 commits into from
May 26, 2021

Conversation

hegocre
Copy link
Collaborator

@hegocre hegocre commented May 25, 2021

Fix for locale changing issues, possible fix #270, tested on Android Emulator API 21 (previously not working, now it does), as well as API 29 and 30 on physical devices, both working.

When creating locales, region code must be a 2 alpha character string as explained here, I imagine the r added on resource folder is an abbreviation for region and should not be used when creating Locales in code.

Also, a bug on AndroidX compat library causes the locale to be overridden to default on API versions 21 to 25 because of DayNight compatibility, so applyOverrideConfiguration must be called with the new context created.

adeekshith
adeekshith previously approved these changes May 25, 2021
Copy link
Owner

@adeekshith adeekshith left a comment

Choose a reason for hiding this comment

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

Thanks a lot @hegocre for this as well as your Spanish and Catalan translations 😃

Calling applyOverrideConfiguration itself fixed the issue on the device I was testing without changing the language code.

Do not see this affecting existing users too but will wait for @spuday90 to review this before merging.

Thank you!!

@hegocre
Copy link
Collaborator Author

hegocre commented May 25, 2021

@adeekshith You are welcome!

I also noticed that some devices neee none, some need one and others need both patches to work correctly.

The problem I see here is that existing users will need to set their language again. Maybe changing PreferencesManager.java to remove the r before creating the locale would be lesw intrusive, but would require you to keep adding the r to new locales.

If you prefer it that way I can change this on my pull request before merging.

Cheers!

@adeekshith
Copy link
Owner

The problem I see here is that existing users will need to set their language again.

Oh I was thinking we were saving the key (lang_code_ca_rES) and not the value (ca-ES). I am wrong then. Thanks for catching.

Maybe changing PreferencesManager.java to remove the r before creating the locale would be lesw intrusive, but would require you to keep adding the r to new locales.

That should do but how about this solution:

  1. Change PreferencesManager::getSelectedLanguageStr() method to remove r before returning (ca-rES to ca-ES).
  2. Or even better, we can write code to migrate existing r version to the non-r version in the shared preferences PreferencesManager::init() method (init() method executed when the PreferencesManager object is created). Advantage with this method is, we can remove the migration code after a few months without affecting anyone.

@adeekshith adeekshith self-requested a review May 25, 2021 16:02
@adeekshith
Copy link
Owner

Awesome! Your fix (186f302) looks good @hegocre
Will test it in a while and merge 😃

Thank you!!

@hegocre
Copy link
Collaborator Author

hegocre commented May 25, 2021

You're welcome!

Amazing work with this app!

@adeekshith adeekshith merged commit fda4361 into adeekshith:main May 26, 2021
adeekshith added a commit that referenced this pull request Jun 6, 2021
* Russian translations by [crlf](https //translate.watomatic.app/profile/crlf) (#289)
* Macedonian translations by [Soma Dima](https //translate.watomatic.app/profile/somadima) (#289)
* Spanish translations by [Diego Muñoz](https //translate.watomatic.app/profile/dmzu) and [Benet R. i Camps](https //translate.watomatic.app/profile/bennybeat) (#289)
* Finnish translations by [Rynach](https //translate.watomatic.app/profile/rynach) (#289)
* Tamil translations by [HarishBeWe](https //translate.watomatic.app/profile/harishbewe) (#289)
* New Polish translations by [Patryk Prawada](https //translate.watomatic.app/profile/gempxplay) (#289)
* German translations by [daengi](https //translate.watomatic.app/profile/daengi) (#289)
* Basque translations by [Joxe Rojas](https //translate.watomatic.app/profile/joxerg) (#289)
* Turkish translations by [Tansel Karakaya](https //translate.watomatic.app/profile/karakaya.tansel) (#289)
* Catalan translations by [Benet R. i Camps](https //translate.watomatic.app/profile/bennybeat) (#289)
* Prevent Watomatic service from getting killed in the background on some devices by @spuday90 (#291)
* Fix language change setting does not work on first click by @hegocre (#298)
* Fixed an issue where language switching is not working on all devices by @hegocre (#292)
* Show settings button in main screen without hiding in the menu (#294)
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.

Language does not change on a few devices
3 participants