From f3f470a977a2146dc3d95ad7ebd6509941318f3e Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Sun, 2 Aug 2020 18:48:10 -0700 Subject: [PATCH] For #13140: Use concept-menu for saved logins menu (#13143) --- app/build.gradle | 1 + .../logins/SavedLoginsSortingStrategyMenu.kt | 77 +++++++++------- .../logins/fragment/SavedLoginsFragment.kt | 58 ++++-------- .../java/org/mozilla/fenix/utils/Settings.kt | 31 +++---- app/src/main/res/values/dimens.xml | 1 + app/src/main/res/values/styles.xml | 3 + .../SavedLoginsSortingStrategyMenuTest.kt | 88 +++++++++++++++++++ 7 files changed, 167 insertions(+), 92 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenuTest.kt diff --git a/app/build.gradle b/app/build.gradle index 6ad80e02a..3f86afe6a 100644 --- 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 diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenu.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSortingStrategyMenu.kt index 8ac54dc81..8e5fbe21d 100644 --- 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 { + 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)) } } diff --git 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 index 3975bb0fe..fe2431ae0 100644 --- 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" - } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index cac21f0b5..910a73035 100644 --- 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 } } } diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index a9bfbf4d2..47bc4e334 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -10,6 +10,7 @@ 112dp 314dp 8dp + 8dp 7dp 56dp 16dp diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 9b89fb6da..f71bd9753 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -250,6 +250,9 @@ +