Add translation by country

[VSeryi]

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_local requests, 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 😉).

[VSeryi]

Added a commit (Add poster translations) to fix #797

EDIT: Splitted in other Pull Request #823

[michaldrabik]

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:

1. In SettingsRepository I 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

[VSeryi]

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:

1. In SettingsRepository I 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

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).

[michaldrabik]

Thinking this should actually be converted into property to avoid creating a new object all the time.

[michaldrabik]

It would probably be cleaner to just get rid of this method from TranslationRepository and get locale from settings repository. Wherever TranslationRepository is injected.

[michaldrabik]

These checks (and also the ones with != ) will need to be updated because now it will only check against "en-us". And for users from ex. United Kingdom (en_GB) it would always try to load english translation which does not make sense really.

Was also thinking of getting rid of these checks accross the code and put them into TranslationRepository itself. If english language is requested return null or empty list. 🤔