Support Per-app language preferences#579
Support Per-app language preferences#579314systems wants to merge 17 commits intoVREMSoftwareDevelopment:mainfrom
Conversation
4d2ae95 to
286c209
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements per-app language preferences by migrating from a custom locale management approach to Android's native AppCompatDelegate API. The changes align with Android's modern language preference system introduced in API 33+, enabling users to select a language specifically for this app independent of the system language.
Key changes:
- Migrated from manual context creation with custom locale to AppCompatDelegate.setApplicationLocales()
- Removed language change tracking from MainReload since AppCompat handles activity recreation automatically
- Updated locale utility functions to use BCP-47 language tags and improved Chinese locale handling with script-based differentiation
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/kotlin/com/vrem/wifianalyzer/MainActivity.kt | Removed attachBaseContext override; added onSharedPreferenceChanged logic to handle language changes via AppCompatDelegate; added syncLanguage call in onCreate |
| app/src/main/kotlin/com/vrem/wifianalyzer/MainReload.kt | Removed language locale tracking since AppCompat handles locale changes automatically |
| app/src/main/kotlin/com/vrem/wifianalyzer/settings/Settings.kt | Replaced languageLocale() with appLocale() using AppCompatDelegate.getApplicationLocales(); added syncLanguage() method |
| app/src/main/kotlin/com/vrem/wifianalyzer/settings/LanguagePreference.kt | Updated to support "System default" option with empty string tag; refactored data() to accept Context |
| app/src/main/kotlin/com/vrem/wifianalyzer/settings/CountryPreference.kt | Updated method call from languageLocale() to appLocale(); renamed utility function reference |
| app/src/main/kotlin/com/vrem/wifianalyzer/wifi/channelavailable/ChannelAvailableFragment.kt | Updated method call from languageLocale() to appLocale() |
| app/src/main/kotlin/com/vrem/wifianalyzer/vendor/model/VendorService.kt | Changed uppercase() to use Locale.ROOT for locale-independent string operations |
| app/src/main/kotlin/com/vrem/util/LocaleUtils.kt | Major refactor: added BCP-47 language tag support, improved Chinese locale handling with script detection, renamed functions for clarity, removed legacy underscore format generation |
| app/src/main/kotlin/com/vrem/util/CompatUtils.kt | Removed createContext() extension function no longer needed with AppCompat |
| app/src/main/kotlin/com/vrem/util/StringUtils.kt | Added titlecaseFirst() extension function for proper title casing of locale display names |
| app/src/main/AndroidManifest.xml | Added AppLocalesMetadataHolderService configuration with autoStoreLocales enabled |
| app/src/main/res/resources.properties | Added unqualifiedResLocale=en-US to specify base locale for resources |
| app/build.gradle | Added generateLocaleConfig=true for automatic locale configuration generation |
| app/src/main/res/values*/strings.xml | Added "system_default" string translations in all supported languages |
| app/src/test/kotlin/**/*Test.kt | Updated tests to reflect API changes: renamed method calls, updated locale handling tests, removed language tracking tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This is a promising improvement, but it does introduce some compatibility considerations. Per‑app language support is only native on Android 13+ (API 33). Since WiFiAnalyzer still supports API 24+, devices on API 24–32 rely on AppCompat’s fallback implementation, which can behave differently across OEMs and Android versions. This may affect Activity recreation, UI stability, and how reliably the selected language is applied. To ensure a smooth transition for all users, this change would benefit from broader testing on real devices in addition to emulator checks, especially across older API levels. It may also be worth considering whether this migration is better aligned with a future release where support for older Android versions is reduced. Could you also share which devices and API levels you tested this on? That would help us better understand the current coverage and identify any gaps. |
|
Thank you for the contribution. |
VREMSoftwareDevelopment
left a comment
There was a problem hiding this comment.
Thank you for the contribution.
The CI pipeline is failing because this change reduces code coverage.
The project requires that coverage does not decrease, so additional unit tests are needed for the new or modified logic.
Please update the PR with the necessary tests so the coverage check can pass.
|
Thank you for the review, and sorry for the late reply. At the moment, I’m not able to dedicate sufficient time to Android development. If possible, I would appreciate it if you could allow me some time until the end of May to add the necessary tests and address the coverage issue. Thank you for your understanding. |
|
No problem. Please take whatever time you need, and feel free to update the PR when you're ready. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
============================================
+ Coverage 97.83% 97.84% +0.01%
- Complexity 975 980 +5
============================================
Files 121 121
Lines 2581 2603 +22
Branches 211 222 +11
============================================
+ Hits 2525 2547 +22
Misses 19 19
Partials 37 37 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private fun data(context: Context): List<Data> = | ||
| supportedLanguageTags().map { tag -> | ||
| if (tag.isEmpty()) { | ||
| Data("", context.getString(R.string.system_default)) | ||
| } else { | ||
| val locale = Locale.forLanguageTag(tag) | ||
| Data(tag, locale.getDisplayName(locale).titlecaseFirst(locale)) | ||
| } | ||
| } | ||
|
|
| fixture.onSharedPreferenceChanged(sharedPreferences, languageKey) | ||
| // validate | ||
| verify(scannerService).update() | ||
| assertThat(AppCompatDelegate.getApplicationLocales().toLanguageTags()).isEqualTo(languageTag) | ||
| } |
63d74c6 to
90ad6a6
Compare
add tests for titlecaseFirst extension function in StringUtilsTest
|
maybe done |
Summary
What does this implement/fix?
Does this close any issues?
How was this tested?
Example test commands:
Checklist (required before marking ready)
app/src/test/)Additional context