For #13140: Use concept-menu for saved logins menu (#13143)

2020-08-02 18:48:10 -07:00 · 2020-08-02 18:48:10 -07:00 · f3f470a977
parent ddfb3dfa72
commit f3f470a977
7 changed files with 167 additions and 92 deletions
--- a/app/build.gradle
+++ b/app/build.gradle
@ -410,6 +410,7 @@ dependencies {
    implementation Deps.mozilla_browser_domains
    implementation Deps.mozilla_browser_icons
    implementation Deps.mozilla_browser_menu
+    implementation Deps.mozilla_browser_menu2
    implementation Deps.mozilla_browser_search
    implementation Deps.mozilla_browser_session
    implementation Deps.mozilla_browser_state
--- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenu.kt
+++ b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenu.kt
@ -5,51 +5,68 @@
 package org.mozilla.fenix.settings.logins

 import android.content.Context
-import mozilla.components.browser.menu.BrowserMenuBuilder
-import mozilla.components.browser.menu.item.SimpleBrowserMenuHighlightableItem
+import androidx.annotation.VisibleForTesting
+import mozilla.components.browser.menu2.BrowserMenuController
+import mozilla.components.concept.menu.candidate.HighPriorityHighlightEffect
+import mozilla.components.concept.menu.candidate.TextMenuCandidate
+import mozilla.components.concept.menu.candidate.TextStyle
 import mozilla.components.support.ktx.android.content.getColorFromAttr
 import org.mozilla.fenix.R
-import org.mozilla.fenix.theme.ThemeManager
+import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor

 class SavedLoginsSortingStrategyMenu(
    private val context: Context,
-    private val itemToHighlight: Item,
-    private val onItemTapped: (Item) -> Unit = {}
+    private val savedLoginsInteractor: SavedLoginsInteractor
 ) {
-    sealed class Item {
-        object AlphabeticallySort : Item()
-        object LastUsedSort : Item()
+    enum class Item(val strategyString: String) {
+        AlphabeticallySort("ALPHABETICALLY"),
+        LastUsedSort("LAST_USED");
+
+        companion object {
+            fun fromString(strategyString: String) = when (strategyString) {
+                AlphabeticallySort.strategyString -> AlphabeticallySort
+                LastUsedSort.strategyString -> LastUsedSort
+                else -> AlphabeticallySort
+            }
+        }
    }

-    val menuBuilder by lazy { BrowserMenuBuilder(menuItems) }
+    val menuController by lazy { BrowserMenuController() }

-    private val menuItems by lazy {
-        listOfNotNull(
-            SimpleBrowserMenuHighlightableItem(
-                label = context.getString(R.string.saved_logins_sort_strategy_alphabetically),
-                textColorResource = ThemeManager.resolveAttribute(R.attr.primaryText, context),
-                itemType = Item.AlphabeticallySort,
-                backgroundTint = context.getColorFromAttr(R.attr.colorControlHighlight),
-                isHighlighted = { itemToHighlight == Item.AlphabeticallySort }
+    @VisibleForTesting
+    internal fun menuItems(itemToHighlight: Item): List<TextMenuCandidate> {
+        val textStyle = TextStyle(
+            color = context.getColorFromAttr(R.attr.primaryText)
+        )
+
+        val highlight = HighPriorityHighlightEffect(
+            backgroundTint = context.getColorFromAttr(R.attr.colorControlHighlight)
+        )
+
+        return listOf(
+            TextMenuCandidate(
+                text = context.getString(R.string.saved_logins_sort_strategy_alphabetically),
+                textStyle = textStyle,
+                effect = if (itemToHighlight == Item.AlphabeticallySort) highlight else null
            ) {
-                onItemTapped.invoke(Item.AlphabeticallySort)
+                savedLoginsInteractor.onSortingStrategyChanged(
+                    SortingStrategy.Alphabetically(context.components.publicSuffixList)
+                )
            },
-
-            SimpleBrowserMenuHighlightableItem(
-                label = context.getString(R.string.saved_logins_sort_strategy_last_used),
-                textColorResource = ThemeManager.resolveAttribute(R.attr.primaryText, context),
-                itemType = Item.LastUsedSort,
-                backgroundTint = context.getColorFromAttr(R.attr.colorControlHighlight),
-                isHighlighted = { itemToHighlight == Item.LastUsedSort }
+            TextMenuCandidate(
+                text = context.getString(R.string.saved_logins_sort_strategy_last_used),
+                textStyle = textStyle,
+                effect = if (itemToHighlight == Item.LastUsedSort) highlight else null
            ) {
-                onItemTapped.invoke(Item.LastUsedSort)
+                savedLoginsInteractor.onSortingStrategyChanged(
+                    SortingStrategy.LastUsed
+                )
            }
        )
    }

-    internal fun updateMenu(itemToHighlight: Item) {
-        menuItems.forEach {
-            it.isHighlighted = { itemToHighlight == it.itemType }
-        }
+    fun updateMenu(itemToHighlight: Item) {
+        menuController.submitList(menuItems(itemToHighlight))
    }
 }
--- a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt
@ -22,8 +22,8 @@ import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
 import kotlinx.android.synthetic.main.fragment_saved_logins.view.*
 import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.ObsoleteCoroutinesApi
-import mozilla.components.browser.menu.BrowserMenu
+import mozilla.components.concept.menu.MenuController
+import mozilla.components.concept.menu.Orientation
 import mozilla.components.lib.state.ext.consumeFrom
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
@ -31,7 +31,6 @@ import org.mozilla.fenix.R
 import org.mozilla.fenix.components.StoreProvider
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.redirectToReAuth
-import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.ext.showToolbar
 import org.mozilla.fenix.settings.logins.LoginsAction
@ -51,7 +50,6 @@ class SavedLoginsFragment : Fragment() {
    private lateinit var savedLoginsInteractor: SavedLoginsInteractor
    private lateinit var dropDownMenuAnchorView: View
    private lateinit var sortingStrategyMenu: SavedLoginsSortingStrategyMenu
-    private lateinit var sortingStrategyPopupMenu: BrowserMenu
    private lateinit var toolbarChildContainer: FrameLayout
    private lateinit var sortLoginsMenuRoot: ConstraintLayout
    private lateinit var loginsListController: LoginsListController
@ -121,10 +119,8 @@ class SavedLoginsFragment : Fragment() {
        return view
    }

-    @ObsoleteCoroutinesApi
    @ExperimentalCoroutinesApi
    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        super.onViewCreated(view, savedInstanceState)
        consumeFrom(savedLoginsStore) {
            sortingStrategyMenu.updateMenu(savedLoginsStore.state.highlightedItem)
            savedLoginsListView.update(it)
@ -161,7 +157,7 @@ class SavedLoginsFragment : Fragment() {
        toolbarChildContainer.removeAllViews()
        toolbarChildContainer.visibility = View.GONE
        (activity as HomeActivity).getSupportActionBarAndInflateIfNecessary().setDisplayShowTitleEnabled(true)
-        sortingStrategyPopupMenu.dismiss()
+        sortingStrategyMenu.menuController.dismiss()

        redirectToReAuth(listOf(R.id.loginDetailFragment), findNavController().currentDestination?.id)
        super.onPause()
@ -206,47 +202,27 @@ class SavedLoginsFragment : Fragment() {
    }

    private fun attachMenu() {
-        sortingStrategyPopupMenu = sortingStrategyMenu.menuBuilder.build(requireContext())
-
-        sortLoginsMenuRoot.setOnClickListener {
-            sortLoginsMenuRoot.isActivated = true
-            sortingStrategyPopupMenu.show(
-                anchor = dropDownMenuAnchorView,
-                orientation = BrowserMenu.Orientation.DOWN
-            ) {
+        sortingStrategyMenu.menuController.register(object : MenuController.Observer {
+            override fun onDismiss() {
+                // Deactivate button on dismiss
                sortLoginsMenuRoot.isActivated = false
            }
+        }, view = sortLoginsMenuRoot)
+
+        sortLoginsMenuRoot.setOnClickListener {
+            // Activate button on show
+            sortLoginsMenuRoot.isActivated = true
+            sortingStrategyMenu.menuController.show(
+                anchor = dropDownMenuAnchorView,
+                orientation = Orientation.DOWN
+            )
        }
    }

    private fun setupMenu(itemToHighlight: SavedLoginsSortingStrategyMenu.Item) {
-        sortingStrategyMenu =
-            SavedLoginsSortingStrategyMenu(
-                requireContext(),
-                itemToHighlight
-            ) {
-                when (it) {
-                    SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort -> {
-                        savedLoginsInteractor.onSortingStrategyChanged(
-                            SortingStrategy.Alphabetically(
-                                requireComponents.publicSuffixList
-                            )
-                        )
-                    }
-
-                    SavedLoginsSortingStrategyMenu.Item.LastUsedSort -> {
-                        savedLoginsInteractor.onSortingStrategyChanged(
-                            SortingStrategy.LastUsed
-                        )
-                    }
-                }
-            }
+        sortingStrategyMenu = SavedLoginsSortingStrategyMenu(requireContext(), savedLoginsInteractor)
+        sortingStrategyMenu.updateMenu(itemToHighlight)

        attachMenu()
    }
-
-    companion object {
-        const val SORTING_STRATEGY_ALPHABETICALLY = "ALPHABETICALLY"
-        const val SORTING_STRATEGY_LAST_USED = "LAST_USED"
-    }
 }
--- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
+++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
@ -36,7 +36,6 @@ import org.mozilla.fenix.settings.PhoneFeature
 import org.mozilla.fenix.settings.deletebrowsingdata.DeleteBrowsingDataOnQuitType
 import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu
 import org.mozilla.fenix.settings.logins.SortingStrategy
-import org.mozilla.fenix.settings.logins.fragment.SavedLoginsFragment
 import org.mozilla.fenix.settings.registerOnSharedPreferenceChangeListener
 import java.security.InvalidParameterException

@ -820,36 +819,26 @@ class Settings(private val appContext: Context) : PreferencesHolder {

    private var savedLoginsSortingStrategyString by stringPreference(
        appContext.getPreferenceKey(R.string.pref_key_saved_logins_sorting_strategy),
-        default = SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY
+        default = SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort.strategyString
    )

    val savedLoginsMenuHighlightedItem: SavedLoginsSortingStrategyMenu.Item
-        get() {
-            return when (savedLoginsSortingStrategyString) {
-                SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY -> {
-                    SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort
-                }
-                SavedLoginsFragment.SORTING_STRATEGY_LAST_USED -> {
-                    SavedLoginsSortingStrategyMenu.Item.LastUsedSort
-                }
-                else -> SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort
-            }
-        }
+        get() = SavedLoginsSortingStrategyMenu.Item.fromString(savedLoginsSortingStrategyString)

    var savedLoginsSortingStrategy: SortingStrategy
        get() {
-            return when (savedLoginsSortingStrategyString) {
-                SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY -> SortingStrategy.Alphabetically(
-                    appContext.components.publicSuffixList
-                )
-                SavedLoginsFragment.SORTING_STRATEGY_LAST_USED -> SortingStrategy.LastUsed
-                else -> SortingStrategy.Alphabetically(appContext.components.publicSuffixList)
+            return when (savedLoginsMenuHighlightedItem) {
+                SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort ->
+                    SortingStrategy.Alphabetically(appContext.components.publicSuffixList)
+                SavedLoginsSortingStrategyMenu.Item.LastUsedSort -> SortingStrategy.LastUsed
            }
        }
        set(value) {
            savedLoginsSortingStrategyString = when (value) {
-                is SortingStrategy.Alphabetically -> SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY
-                is SortingStrategy.LastUsed -> SavedLoginsFragment.SORTING_STRATEGY_LAST_USED
+                is SortingStrategy.Alphabetically ->
+                    SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort.strategyString
+                is SortingStrategy.LastUsed ->
+                    SavedLoginsSortingStrategyMenu.Item.LastUsedSort.strategyString
            }
        }
 }
--- a/app/src/main/res/values/dimens.xml
+++ b/app/src/main/res/values/dimens.xml
@ -10,6 +10,7 @@
    <dimen name="mozac_browser_menu_width_min" tools:ignore="UnusedResources">112dp</dimen>
    <dimen name="mozac_browser_menu_width_max" tools:ignore="UnusedResources">314dp</dimen>
    <dimen name="mozac_browser_menu_corner_radius">8dp</dimen>
+    <dimen name="mozac_browser_menu2_corner_radius">8dp</dimen>
    <dimen name="toolbar_elevation">7dp</dimen>
    <dimen name="library_item_height">56dp</dimen>
    <dimen name="library_item_icon_margin_horizontal">16dp</dimen>
--- a/app/src/main/res/values/styles.xml
+++ b/app/src/main/res/values/styles.xml
@ -250,6 +250,9 @@
    <style name="Mozac.Browser.Menu" parent="" tools:ignore="UnusedResources">
        <item name="cardBackgroundColor">?above</item>
    </style>
+    <style name="Mozac.Browser.Menu2" parent="" tools:ignore="UnusedResources">
+        <item name="cardBackgroundColor">?above</item>
+    </style>

    <style name="PrivateTheme" parent="PrivateThemeBase" />

--- a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenuTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenuTest.kt
@ -0,0 +1,88 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.settings.logins
+
+import android.content.Context
+import androidx.appcompat.view.ContextThemeWrapper
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.verify
+import mozilla.components.concept.menu.candidate.HighPriorityHighlightEffect
+import mozilla.components.support.ktx.android.content.getColorFromAttr
+import mozilla.components.support.test.robolectric.testContext
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.R
+import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
+import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu.Item
+import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor
+
+@RunWith(FenixRobolectricTestRunner::class)
+class SavedLoginsSortingStrategyMenuTest {
+
+    private lateinit var context: Context
+    private lateinit var interactor: SavedLoginsInteractor
+    private lateinit var menu: SavedLoginsSortingStrategyMenu
+
+    @Before
+    fun setup() {
+        context = ContextThemeWrapper(testContext, R.style.NormalTheme)
+        interactor = mockk()
+        menu = SavedLoginsSortingStrategyMenu(context, interactor)
+    }
+
+    @Test
+    fun `item enum can be deserialized from string`() {
+        assertEquals(Item.AlphabeticallySort, Item.fromString("ALPHABETICALLY"))
+        assertEquals(Item.LastUsedSort, Item.fromString("LAST_USED"))
+        assertEquals(Item.AlphabeticallySort, Item.fromString("OTHER"))
+    }
+
+    @Test
+    fun `effect is set on alphabetical sort candidate`() {
+        val (name, lastUsed) = menu.menuItems(Item.AlphabeticallySort)
+        assertEquals(
+            HighPriorityHighlightEffect(context.getColorFromAttr(R.attr.colorControlHighlight)),
+            name.effect
+        )
+        assertNull(lastUsed.effect)
+    }
+
+    @Test
+    fun `effect is set on last used sort candidate`() {
+        val (name, lastUsed) = menu.menuItems(Item.LastUsedSort)
+        assertNull(name.effect)
+        assertEquals(
+            HighPriorityHighlightEffect(context.getColorFromAttr(R.attr.colorControlHighlight)),
+            lastUsed.effect
+        )
+    }
+
+    @Test
+    fun `candidates call interactor on click`() {
+        val (name, lastUsed) = menu.menuItems(Item.AlphabeticallySort)
+        every { interactor.onSortingStrategyChanged(any()) } just Runs
+
+        name.onClick()
+        verify {
+            interactor.onSortingStrategyChanged(
+                SortingStrategy.Alphabetically(context.components.publicSuffixList)
+            )
+        }
+
+        lastUsed.onClick()
+        verify {
+            interactor.onSortingStrategyChanged(
+                SortingStrategy.LastUsed
+            )
+        }
+    }
+}