From f2b07196e6accb478c93cbc118864fb7efe83310 Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Sat, 25 Feb 2023 21:15:21 +0100 Subject: [PATCH] Improve language list prioritization. (#3293) Partially addresses #3277 --- .../components/compose/ComposeActivity.kt | 8 +- .../preference/AccountPreferencesFragment.kt | 4 +- .../keylesspalace/tusky/util/LocaleUtils.kt | 90 +++++++++---------- .../tusky/util/LocaleUtilsTest.kt | 82 +++++++++++++++++ 4 files changed, 128 insertions(+), 56 deletions(-) create mode 100644 app/src/test/java/com/keylesspalace/tusky/util/LocaleUtilsTest.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt index b49b5e7f1..7ff0733e1 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt @@ -90,7 +90,7 @@ import com.keylesspalace.tusky.settings.PrefKeys import com.keylesspalace.tusky.util.APP_THEME_DEFAULT import com.keylesspalace.tusky.util.PickMediaFiles import com.keylesspalace.tusky.util.afterTextChanged -import com.keylesspalace.tusky.util.getInitialLanguage +import com.keylesspalace.tusky.util.getInitialLanguages import com.keylesspalace.tusky.util.getLocaleList import com.keylesspalace.tusky.util.getMediaSize import com.keylesspalace.tusky.util.hide @@ -267,7 +267,7 @@ class ComposeActivity : binding.composeScheduleView.setDateTime(composeOptions?.scheduledAt) } - setupLanguageSpinner(getInitialLanguage(composeOptions?.language, accountManager.activeAccount)) + setupLanguageSpinner(getInitialLanguages(composeOptions?.language, accountManager.activeAccount)) setupComposeField(preferences, viewModel.startingText) setupContentWarningField(composeOptions?.contentWarning) setupPollView() @@ -543,7 +543,7 @@ class ComposeActivity : ) } - private fun setupLanguageSpinner(initialLanguage: String) { + private fun setupLanguageSpinner(initialLanguages: List) { binding.composePostLanguageButton.onItemSelectedListener = object : AdapterView.OnItemSelectedListener { override fun onItemSelected(parent: AdapterView<*>, view: View?, position: Int, id: Long) { viewModel.postLanguage = (parent.adapter.getItem(position) as Locale).modernLanguageCode @@ -554,7 +554,7 @@ class ComposeActivity : } } binding.composePostLanguageButton.apply { - adapter = LocaleAdapter(context, android.R.layout.simple_spinner_dropdown_item, getLocaleList(initialLanguage)) + adapter = LocaleAdapter(context, android.R.layout.simple_spinner_dropdown_item, getLocaleList(initialLanguages)) setSelection(0) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/preference/AccountPreferencesFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/preference/AccountPreferencesFragment.kt index 25024d00a..0cbdacba6 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/preference/AccountPreferencesFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/preference/AccountPreferencesFragment.kt @@ -49,7 +49,7 @@ import com.keylesspalace.tusky.settings.makePreferenceScreen import com.keylesspalace.tusky.settings.preference import com.keylesspalace.tusky.settings.preferenceCategory import com.keylesspalace.tusky.settings.switchPreference -import com.keylesspalace.tusky.util.getInitialLanguage +import com.keylesspalace.tusky.util.getInitialLanguages import com.keylesspalace.tusky.util.getLocaleList import com.keylesspalace.tusky.util.getTuskyDisplayName import com.keylesspalace.tusky.util.makeIcon @@ -197,7 +197,7 @@ class AccountPreferencesFragment : PreferenceFragmentCompat(), Injectable { } listPreference { - val locales = getLocaleList(getInitialLanguage(null, accountManager.activeAccount)) + val locales = getLocaleList(getInitialLanguages(null, accountManager.activeAccount)) setTitle(R.string.pref_default_post_language) // Explicitly add "System default" to the start of the list entries = ( diff --git a/app/src/main/java/com/keylesspalace/tusky/util/LocaleUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/LocaleUtils.kt index 316e14d0a..655348b4d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/LocaleUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/LocaleUtils.kt @@ -23,67 +23,57 @@ import java.util.Locale private const val TAG: String = "LocaleUtils" -private fun mergeLocaleListCompat(list: MutableList, localeListCompat: LocaleListCompat) { - for (index in 0 until localeListCompat.size()) { - val locale = localeListCompat[index] - if (locale != null && list.none { locale.language == it.language }) { - list.add(locale) - } +private fun LocaleListCompat.toList(): List { + val list = mutableListOf() + for (index in 0 until this.size()) { + this[index]?.let { list.add(it) } } + return list } // Ensure that the locale whose code matches the given language is first in the list -private fun ensureLanguageIsFirst(locales: MutableList, language: String) { - var currentLocaleIndex = locales.indexOfFirst { it.language == language } - if (currentLocaleIndex < 0) { - // Recheck against modern language codes - // This should only happen when replying or when the per-account post language is set - // to a modern code - currentLocaleIndex = locales.indexOfFirst { it.modernLanguageCode == language } - +private fun ensureLanguagesAreFirst(locales: MutableList, languages: List) { + for (language in languages.reversed()) { + // Iterate prioritized languages in reverse to retain the order once bubbled to the top + var currentLocaleIndex = locales.indexOfFirst { it.language == language } if (currentLocaleIndex < 0) { - // This can happen when: - // - Your per-account posting language is set to one android doesn't know (e.g. toki pona) - // - Replying to a post in a language android doesn't know - locales.add(0, Locale(language)) - Log.w(TAG, "Attempting to use unknown language tag '$language'") - return - } - } + // Recheck against modern language codes + // This should only happen when replying or when the per-account post language is set + // to a modern code + currentLocaleIndex = locales.indexOfFirst { it.modernLanguageCode == language } - if (currentLocaleIndex > 0) { - // Move preselected locale to the top - locales.add(0, locales.removeAt(currentLocaleIndex)) + if (currentLocaleIndex < 0) { + // This can happen when: + // - Your per-account posting language is set to one android doesn't know (e.g. toki pona) + // - Replying to a post in a language android doesn't know + locales.add(0, Locale(language)) + Log.w(TAG, "Attempting to use unknown language tag '$language'") + continue + } + } + + if (currentLocaleIndex > 0) { + // Move preselected locale to the top + locales.add(0, locales.removeAt(currentLocaleIndex)) + } } } -fun getInitialLanguage(language: String? = null, activeAccount: AccountEntity? = null): String { - return if (language.isNullOrEmpty()) { - // Account-specific language set on the server - if (activeAccount?.defaultPostLanguage?.isNotEmpty() == true) { - activeAccount.defaultPostLanguage - } else { - // Setting the application ui preference sets the default locale - AppCompatDelegate.getApplicationLocales()[0]?.language - ?: Locale.getDefault().language - } - } else { - language - } +fun getInitialLanguages(language: String? = null, activeAccount: AccountEntity? = null): List { + val selected = listOfNotNull(language, activeAccount?.defaultPostLanguage) + val system = AppCompatDelegate.getApplicationLocales().toList() + + LocaleListCompat.getDefault().toList() + + return (selected + system.map { it.language }).distinct().filter { it.isNotEmpty() } } -fun getLocaleList(initialLanguage: String): List { - val locales = mutableListOf() - mergeLocaleListCompat(locales, AppCompatDelegate.getApplicationLocales()) // configured app languages first - mergeLocaleListCompat(locales, LocaleListCompat.getDefault()) // then configured system languages - locales.addAll( // finally, other languages +fun getLocaleList(initialLanguages: List): List { + val locales = Locale.getAvailableLocales().filter { // Only "base" languages, "en" but not "en_DK" - Locale.getAvailableLocales().filter { - it.country.isNullOrEmpty() && - it.script.isNullOrEmpty() && - it.variant.isNullOrEmpty() - }.sortedBy { it.displayName } - ) - ensureLanguageIsFirst(locales, initialLanguage) + it.country.isNullOrEmpty() && + it.script.isNullOrEmpty() && + it.variant.isNullOrEmpty() + }.sortedBy { it.displayName }.toMutableList() + ensureLanguagesAreFirst(locales, initialLanguages) return locales } diff --git a/app/src/test/java/com/keylesspalace/tusky/util/LocaleUtilsTest.kt b/app/src/test/java/com/keylesspalace/tusky/util/LocaleUtilsTest.kt new file mode 100644 index 000000000..db287cca9 --- /dev/null +++ b/app/src/test/java/com/keylesspalace/tusky/util/LocaleUtilsTest.kt @@ -0,0 +1,82 @@ +package com.keylesspalace.tusky.util + +import androidx.appcompat.app.AppCompatDelegate +import androidx.core.os.LocaleListCompat +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.keylesspalace.tusky.db.AccountEntity +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito +import org.robolectric.annotation.Config + +@Config(sdk = [28]) +@RunWith(AndroidJUnit4::class) +class LocaleUtilsTest { + @Test + fun initialLanguagesContainReplySelectedAppAndSystem() { + val expectedLanguages = arrayOf("yi", "tok", "da", "fr", "sv", "kab") + val languages = getMockedInitialLanguages(expectedLanguages) + Assert.assertArrayEquals(expectedLanguages, languages.subList(0, expectedLanguages.size).toTypedArray()) + } + + @Test + fun whenReplyLanguageIsNull_DefaultLanguageIsFirst() { + val defaultLanguage = "tok" + val languages = getMockedInitialLanguages(arrayOf(null, defaultLanguage, "da", "fr", "sv", "kab")) + Assert.assertEquals(defaultLanguage, languages[0]) + } + + @Test + fun initialLanguagesAreDistinct() { + val defaultLanguage = "da" + val languages = getMockedInitialLanguages(arrayOf(defaultLanguage, defaultLanguage, "fr", defaultLanguage, "kab", defaultLanguage)) + Assert.assertEquals(1, languages.count { it == defaultLanguage }) + } + + @Test + fun initialLanguageDeduplicationDoesNotReorder() { + val defaultLanguage = "da" + + Assert.assertEquals( + defaultLanguage, + getMockedInitialLanguages(arrayOf(defaultLanguage, defaultLanguage, "fr", defaultLanguage, "kab", defaultLanguage))[0] + ) + Assert.assertEquals( + defaultLanguage, + getMockedInitialLanguages(arrayOf(null, defaultLanguage, "fr", defaultLanguage, "kab", defaultLanguage))[0] + ) + } + + @Test + fun emptyInitialLanguagesAreDropped() { + val languages = getMockedInitialLanguages(arrayOf("", "", "fr", "", "kab", "")) + Assert.assertFalse(languages.any { it.isEmpty() }) + } + + private fun getMockedInitialLanguages(configuredLanguages: Array): List { + val appLanguages = LocaleListCompat.forLanguageTags(configuredLanguages.slice(2 until 4).joinToString(",")) + val systemLanguages = LocaleListCompat.forLanguageTags(configuredLanguages.slice(4 until configuredLanguages.size).joinToString(",")) + + Mockito.mockStatic(AppCompatDelegate::class.java).use { appCompatDelegate -> + appCompatDelegate.`when` { AppCompatDelegate.getApplicationLocales() }.thenReturn(appLanguages) + + Mockito.mockStatic(LocaleListCompat::class.java).use { localeListCompat -> + localeListCompat.`when` { LocaleListCompat.getDefault() }.thenReturn(systemLanguages) + + return getInitialLanguages( + configuredLanguages[0], + AccountEntity( + id = 0, + domain = "foo.bar", + accessToken = "", + clientId = null, + clientSecret = null, + isActive = true, + defaultPostLanguage = configuredLanguages[1] ?: "", + ) + ) + } + } + } +}