showly-2.0
showly-2.0 copied to clipboard
Add translation by country
Overview
This Pull Request introduces a version for Showly 2.0 that allows movie and series translations to be displayed based on the user's configured country.
It resolved Issues as #546 and #496
Changes
- Changed logic to filter translations by language and country:
- If not found, it falls back to language-specific translations.
- If not found, it falls back to the default language.
Points for Improvement
- Currently using
Pair<String, String>for language representation. A custom class could be more suitable for future scalability and clarity. I used it due to uncertainty about the appropriate placement and naming for a custom class.
Suggestions
- It would be beneficial to explore methods to reduce
only_localrequests, aiming to translate everything upfront and avoid the need for user-triggered translations.
Acknowledgements
A huge thank you to everyone involved in developing Showly 2.0. It's my go-to app every day, even though the ads are becoming more frequent (just a little joke 😉).
Added a commit (Add poster translations) to fix #797
EDIT: Splitted in other Pull Request #823
In general this is good initiative and wanted to add it myself at some point finally. 👍🏻
If you are up for some refactors I would have some propositions as I see it:
- In
SettingsRepositoryI would like to end up only with a single AppLocale instead of both language and country:
data class AppLocale(
val language: AppLanguage,
val country: AppCountry,
) {
companion object {
fun default() = AppLocale(
language = AppLanguage.getDefault(),
country = AppCountry.getDefault(),
)
}
fun getLocaleCode() = "${language.code}-${country.code}".lowercase()
}
AppLocale, AppCountry and AppLanguage could all be moved into ui-model/locale package
SettingsRepository would only have this instead of language and country
companion object Key {
private const val LOCALE = "KEY_LOCALE"
...
}
...
var locale: AppLocale
get() {
val value = preferences.getString(LOCALE, null) ?: AppLocale.default().getLocaleCode()
val (languageCode, countryCode) = value.split("-")
return AppLocale(
appLanguage = AppLanguage.fromCode(languageCode),
appCountry = AppCountry.fromCode(countryCode),
)
}
set(value) = preferences.edit(true) { putString(LOCALE, value.getLocaleCode()) }
When having this I would continue the further refactors. It would clear things a bit IMHO. wdyt
In general this is good initiative and wanted to add it myself at some point finally. 👍🏻
If you are up for some refactors I would have some propositions as I see it:
- In
SettingsRepositoryI would like to end up only with a single AppLocale instead of both language and country:data class AppLocale( val language: AppLanguage, val country: AppCountry, ) { companion object { fun default() = AppLocale( language = AppLanguage.getDefault(), country = AppCountry.getDefault(), ) } fun getLocaleCode() = "${language.code}-${country.code}".lowercase() }
AppLocale,AppCountryandAppLanguagecould all be moved intoui-model/localepackage
SettingsRepositorywould only have this instead oflanguageandcountrycompanion object Key { private const val LOCALE = "KEY_LOCALE" ... } ... var locale: AppLocale get() { val value = preferences.getString(LOCALE, null) ?: AppLocale.default().getLocaleCode() val (languageCode, countryCode) = value.split("-") return AppLocale( appLanguage = AppLanguage.fromCode(languageCode), appCountry = AppCountry.fromCode(countryCode), ) } set(value) = preferences.edit(true) { putString(LOCALE, value.getLocaleCode()) }When having this I would continue the further refactors. It would clear things a bit IMHO. wdyt
Completely agree (the Pair didn't leave a very readable code with the first and second).
I've updated the pull request with the changes you mentioned with the only difference of separating the locale with _ instead of - because it's more similar to the way Android use it (without taking into account the uppercase country).