From 057e28d4e4b4aeab6a454b99c305adb5096763f7 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 30 Mar 2020 09:48:40 -0700 Subject: [PATCH] Part 5: Refactor HomeMenu<->HomeFragment interaction This refactor "reverses" relationship between these two classes, allowing HomeMenu to inform its parent, HomeFragment, of any changes to the menu. Once that's in place, we start observing account manager changes (once its ready) for account problems. This solves two problems: - initialization of the account manager is no longer necessary to build a home menu - home menu now starts observing changes to the account manager's state (before it was static) --- .../org/mozilla/fenix/home/HomeFragment.kt | 32 +++--- .../java/org/mozilla/fenix/home/HomeMenu.kt | 103 ++++++++++++------ 2 files changed, 87 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 2eaeb1fb5..9de9f019e 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -64,7 +64,7 @@ import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import mozilla.appservices.places.BookmarkRoot -import mozilla.components.browser.menu.ext.getHighlight +import mozilla.components.browser.menu.view.MenuButton import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.store.BrowserStore @@ -107,6 +107,7 @@ import org.mozilla.fenix.theme.ThemeManager import org.mozilla.fenix.utils.FragmentPreDrawManager import org.mozilla.fenix.utils.allowUndo import org.mozilla.fenix.whatsnew.WhatsNew +import java.lang.ref.WeakReference import kotlin.math.abs import kotlin.math.min @@ -321,20 +322,13 @@ class HomeFragment : Fragment() { } } - with(view.menuButton) { - menuBuilder = createHomeMenu(context!!).menuBuilder + createHomeMenu(context!!, WeakReference(view.menuButton)) - val primaryTextColor = ContextCompat.getColor( - context, - ThemeManager.resolveAttribute(R.attr.primaryText, context) - ) + view.menuButton.setColorFilter(ContextCompat.getColor( + context!!, + ThemeManager.resolveAttribute(R.attr.primaryText, context!!) + )) - setColorFilter(primaryTextColor) - - // Immediately check for `What's new` highlight. If home items that change over time - // are added, this will need to be called repeatedly. - setHighlight(menuBuilder?.items?.getHighlight()) - } view.toolbar.compoundDrawablePadding = view.resources.getDimensionPixelSize(R.dimen.search_bar_search_engine_icon_padding) view.toolbar_wrapper.setOnClickListener { @@ -607,8 +601,10 @@ class HomeFragment : Fragment() { } @SuppressWarnings("ComplexMethod") - private fun createHomeMenu(context: Context): HomeMenu { - return HomeMenu(context) { + private fun createHomeMenu(context: Context, menuButtonView: WeakReference) = HomeMenu( + this, + context, + onItemTapped = { when (it) { HomeMenu.Item.Settings -> { invokePendingDeleteJobs() @@ -676,8 +672,10 @@ class HomeFragment : Fragment() { ) } } - } - } + }, + onHighlightPresent = { menuButtonView.get()?.setHighlight(it) }, + onMenuBuilderChanged = { menuButtonView.get()?.menuBuilder = it } + ) private fun subscribeToTabCollections(): Observer> { return Observer> { diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt index 9e6e2b3c3..32de42a9f 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt @@ -6,12 +6,18 @@ package org.mozilla.fenix.home import android.content.Context import androidx.core.content.ContextCompat.getColor +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner import mozilla.components.browser.menu.BrowserMenuBuilder import mozilla.components.browser.menu.BrowserMenuHighlight +import mozilla.components.browser.menu.ext.getHighlight import mozilla.components.browser.menu.item.BrowserMenuCategory import mozilla.components.browser.menu.item.BrowserMenuDivider import mozilla.components.browser.menu.item.BrowserMenuHighlightableItem import mozilla.components.browser.menu.item.BrowserMenuImageText +import mozilla.components.concept.sync.AccountObserver +import mozilla.components.concept.sync.AuthType +import mozilla.components.concept.sync.OAuthAccount import mozilla.components.support.ktx.android.content.getColorFromAttr import org.mozilla.fenix.R import org.mozilla.fenix.ext.components @@ -20,8 +26,11 @@ import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.whatsnew.WhatsNew class HomeMenu( + private val lifecycleOwner: LifecycleOwner, private val context: Context, - private val onItemTapped: (Item) -> Unit = {} + private val onItemTapped: (Item) -> Unit = {}, + private val onMenuBuilderChanged: (BrowserMenuBuilder) -> Unit = {}, + private val onHighlightPresent: (BrowserMenuHighlight) -> Unit = {} ) { sealed class Item { object WhatsNew : Item() @@ -34,9 +43,6 @@ class HomeMenu( object Sync : Item() } - val menuBuilder by lazy { BrowserMenuBuilder(menuItems) } - - private val hasAccountProblem get() = context.components.backgroundServices.accountManager.accountNeedsReauth() private val primaryTextColor = ThemeManager.resolveAttribute(R.attr.primaryText, context) private val syncDisconnectedColor = ThemeManager.resolveAttribute(R.attr.syncDisconnected, context) @@ -45,21 +51,7 @@ class HomeMenu( private val menuCategoryTextColor = ThemeManager.resolveAttribute(R.attr.menuCategoryText, context) - private val menuItems by lazy { - - val reconnectToSyncItem = BrowserMenuHighlightableItem( - context.getString(R.string.sync_reconnect), - R.drawable.ic_sync_disconnected, - iconTintColorResource = syncDisconnectedColor, - textColorResource = primaryTextColor, - highlight = BrowserMenuHighlight.HighPriority( - backgroundTint = syncDisconnectedBackgroundColor - ), - isHighlighted = { true } - ) { - onItemTapped.invoke(Item.Sync) - } - + private val coreMenuItems by lazy { val whatsNewItem = BrowserMenuHighlightableItem( context.getString(R.string.browser_menu_whats_new), R.drawable.ic_whats_new, @@ -103,16 +95,7 @@ class HomeMenu( onItemTapped.invoke(Item.Help) } - val quitItem = BrowserMenuImageText( - context.getString(R.string.delete_browsing_data_on_quit_action), - R.drawable.ic_exit, - primaryTextColor - ) { - onItemTapped.invoke(Item.Quit) - } - - val items = listOfNotNull( - if (hasAccountProblem) reconnectToSyncItem else null, + listOfNotNull( whatsNewItem, BrowserMenuDivider(), BrowserMenuCategory( @@ -125,8 +108,66 @@ class HomeMenu( settingsItem, helpItem, if (Settings.getInstance(context).shouldDeleteBrowsingDataOnQuit) quitItem else null - ) + ).also { items -> + items.getHighlight()?.let { onHighlightPresent(it) } + } + } - items + init { + // Report initial state. + onMenuBuilderChanged(BrowserMenuBuilder(coreMenuItems)) + + // Observe account state changes, and update menu item builder with a new set of items. + context.components.backgroundServices.accountManagerAvailableQueue.runIfReadyOrQueue { + // This task isn't relevant if our parent fragment isn't around anymore. + if (lifecycleOwner.lifecycle.currentState == Lifecycle.State.DESTROYED) { + return@runIfReadyOrQueue + } + context.components.backgroundServices.accountManager.register(object : AccountObserver { + override fun onAuthenticationProblems() { + onMenuBuilderChanged(BrowserMenuBuilder( + listOf(reconnectToSyncItem) + coreMenuItems + )) + } + + override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { + onMenuBuilderChanged(BrowserMenuBuilder( + coreMenuItems + )) + } + + override fun onLoggedOut() { + onMenuBuilderChanged(BrowserMenuBuilder( + coreMenuItems + )) + } + }, lifecycleOwner) + } + } + + // 'Reconnect' and 'Quit' items aren't needed most of the time, so we'll only create the if necessary. + private val reconnectToSyncItem by lazy { + BrowserMenuHighlightableItem( + context.getString(R.string.sync_reconnect), + R.drawable.ic_sync_disconnected, + iconTintColorResource = syncDisconnectedColor, + textColorResource = primaryTextColor, + highlight = BrowserMenuHighlight.HighPriority( + backgroundTint = syncDisconnectedBackgroundColor + ), + isHighlighted = { true } + ) { + onItemTapped.invoke(Item.Sync) + } + } + + private val quitItem by lazy { + BrowserMenuImageText( + context.getString(R.string.delete_browsing_data_on_quit_action), + R.drawable.ic_exit, + primaryTextColor + ) { + onItemTapped.invoke(Item.Quit) + } } }