From b3d99c6bba03196c2f335d6ec00193188a2c668a Mon Sep 17 00:00:00 2001 From: Mihai Branescu Date: Tue, 28 Jan 2020 19:22:04 +0200 Subject: [PATCH] For #7679 - Replaced comparison by reference with value, added check for default in order to avoid double checkmark (example: default + English) (#7729) --- .../fenix/settings/advanced/LocaleAdapter.kt | 42 ++++++++++------ .../advanced/LocaleManagerExtension.kt | 4 ++ .../settings/advanced/LocaleAdapterTest.kt | 48 +++++++++++++++++++ .../advanced/LocaleManagerExtensionTest.kt | 22 +++++++++ 4 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleAdapterTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleAdapter.kt b/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleAdapter.kt index 1faba7e62..1965bb48f 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleAdapter.kt @@ -7,12 +7,14 @@ package org.mozilla.fenix.settings.advanced import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.annotation.VisibleForTesting import androidx.core.view.isVisible import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.locale_settings_item.view.locale_selected_icon import kotlinx.android.synthetic.main.locale_settings_item.view.locale_subtitle_text import kotlinx.android.synthetic.main.locale_settings_item.view.locale_title_text +import mozilla.components.support.locale.LocaleManager import org.mozilla.fenix.R import java.util.Locale @@ -30,13 +32,13 @@ class LocaleAdapter(private val interactor: LocaleSettingsViewInteractor) : return when (viewType) { ItemType.DEFAULT.ordinal -> SystemLocaleViewHolder( view, - interactor, - selectedLocale + selectedLocale, + interactor ) ItemType.LOCALE.ordinal -> LocaleViewHolder( view, - interactor, - selectedLocale + selectedLocale, + interactor ) else -> throw IllegalStateException("ViewType $viewType does not match to a ViewHolder") } @@ -99,9 +101,9 @@ class LocaleAdapter(private val interactor: LocaleSettingsViewInteractor) : class LocaleViewHolder( view: View, - private val interactor: LocaleSettingsViewInteractor, - private val selectedLocale: Locale -) : BaseLocaleViewHolder(view) { + selectedLocale: Locale, + private val interactor: LocaleSettingsViewInteractor +) : BaseLocaleViewHolder(view, selectedLocale) { private val icon = view.locale_selected_icon private val title = view.locale_title_text private val subtitle = view.locale_subtitle_text @@ -110,7 +112,7 @@ class LocaleViewHolder( // capitalisation is done using the rules of the appropriate locale (endonym and exonym) title.text = locale.getDisplayName(locale).capitalize(locale) subtitle.text = locale.displayName.capitalize(Locale.getDefault()) - icon.isVisible = locale === selectedLocale + icon.isVisible = isCurrentLocaleSelected(locale, isDefault = false) itemView.setOnClickListener { interactor.onLocaleSelected(locale) @@ -120,9 +122,9 @@ class LocaleViewHolder( class SystemLocaleViewHolder( view: View, - private val interactor: LocaleSettingsViewInteractor, - private val selectedLocale: Locale -) : BaseLocaleViewHolder(view) { + selectedLocale: Locale, + private val interactor: LocaleSettingsViewInteractor +) : BaseLocaleViewHolder(view, selectedLocale) { private val icon = view.locale_selected_icon private val title = view.locale_title_text private val subtitle = view.locale_subtitle_text @@ -130,15 +132,27 @@ class SystemLocaleViewHolder( override fun bind(locale: Locale) { title.text = itemView.context.getString(R.string.default_locale_text) subtitle.visibility = View.GONE - icon.isVisible = locale === selectedLocale - + icon.isVisible = isCurrentLocaleSelected(locale, isDefault = true) itemView.setOnClickListener { interactor.onDefaultLocaleSelected() } } } -abstract class BaseLocaleViewHolder(view: View) : RecyclerView.ViewHolder(view) { +abstract class BaseLocaleViewHolder( + view: View, + private val selectedLocale: Locale +) : RecyclerView.ViewHolder(view) { + + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + internal fun isCurrentLocaleSelected(locale: Locale, isDefault: Boolean): Boolean { + return if (isDefault) { + locale == selectedLocale && LocaleManager.isDefaultLocaleSelected(itemView.context) + } else { + locale == selectedLocale && !LocaleManager.isDefaultLocaleSelected(itemView.context) + } + } + abstract fun bind(locale: Locale) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtension.kt b/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtension.kt index 17da2e811..aea61b51c 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtension.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtension.kt @@ -47,3 +47,7 @@ fun LocaleManager.getSelectedLocale( supportedMatch ?: defaultLocale } } + +fun LocaleManager.isDefaultLocaleSelected(context: Context): Boolean { + return getCurrentLocale(context) == null +} diff --git a/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleAdapterTest.kt new file mode 100644 index 000000000..f883e9c5b --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleAdapterTest.kt @@ -0,0 +1,48 @@ +package org.mozilla.fenix.settings.advanced + +import android.content.Context +import android.view.View +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import mozilla.components.support.locale.LocaleManager +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.util.Locale + +class LocaleAdapterTest { + + private val selectedLocale = Locale("en", "UK") + private val view: View = mockk(relaxed = true) + private val context: Context = mockk(relaxed = true) + + private val localeViewHolder: BaseLocaleViewHolder = + object : BaseLocaleViewHolder(view, selectedLocale) { + + override fun bind(locale: Locale) { + // not required + } + } + + @Before + fun setup() { + every { view.context } returns context + } + + @Test + fun `verify selected locale checker returns true`() { + mockkStatic("org.mozilla.fenix.settings.advanced.LocaleManagerExtensionKt") + every { LocaleManager.isDefaultLocaleSelected(context) } returns false + + assertTrue(localeViewHolder.isCurrentLocaleSelected(selectedLocale, isDefault = false)) + } + + @Test + fun `verify default locale checker returns true`() { + mockkStatic("org.mozilla.fenix.settings.advanced.LocaleManagerExtensionKt") + every { LocaleManager.isDefaultLocaleSelected(context) } returns true + + assertTrue(localeViewHolder.isCurrentLocaleSelected(selectedLocale, isDefault = true)) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtensionTest.kt b/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtensionTest.kt index ed9a4a5be..88710a5bf 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtensionTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/advanced/LocaleManagerExtensionTest.kt @@ -11,6 +11,7 @@ import io.mockk.mockkObject import io.mockk.mockkStatic import mozilla.components.support.locale.LocaleManager import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -42,6 +43,27 @@ class LocaleManagerExtensionTest { assertTrue(list.isNotEmpty()) } + @Test + @Config(qualifiers = "en-rUS") + fun `default locale selected`() { + val context: Context = mockk() + mockkObject(LocaleManager) + every { LocaleManager.getCurrentLocale(context) } returns null + + assertTrue(LocaleManager.isDefaultLocaleSelected(context)) + } + + @Test + @Config(qualifiers = "en-rUS") + fun `custom locale selected`() { + val context: Context = mockk() + mockkObject(LocaleManager) + val selectedLocale = Locale("en", "UK") + every { LocaleManager.getCurrentLocale(context) } returns selectedLocale + + assertFalse(LocaleManager.isDefaultLocaleSelected(context)) + } + @Test @Config(qualifiers = "en-rUS") fun `match current stored locale string with a Locale from our list`() {