Skip to content

Commit

Permalink
Closes mozilla-mobile#11290: Refactor ToolbarMenu to use browser store
Browse files Browse the repository at this point in the history
  • Loading branch information
csadilek committed Dec 8, 2020
1 parent 04736ed commit d1345e8
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ class BrowserToolbarView(
val menuToolbar: ToolbarMenu
if (isCustomTabSession) {
menuToolbar = CustomTabToolbarMenu(
this,
sessionManager,
customTabSession?.id,
context = this,
store = components.core.store,
sessionId = customTabSession?.id,
shouldReverseItems = toolbarPosition == ToolbarPosition.TOP,
onItemTapped = {
it.performHapticIfNeeded(view)
Expand All @@ -179,7 +179,6 @@ class BrowserToolbarView(
interactor.onBrowserToolbarMenuItemTapped(it)
},
lifecycleOwner = lifecycleOwner,
sessionManager = sessionManager,
store = components.core.store,
bookmarksStorage = bookmarkStorage,
isPinningSupported = isPinningSupported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ package org.mozilla.fenix.components.toolbar

import android.content.Context
import androidx.annotation.ColorRes
import androidx.annotation.VisibleForTesting
import androidx.core.content.ContextCompat.getColor
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.launch
import mozilla.components.browser.menu.BrowserMenuHighlight
import mozilla.components.browser.menu.WebExtensionBrowserMenuBuilder
Expand All @@ -19,13 +23,15 @@ import mozilla.components.browser.menu.item.BrowserMenuImageSwitch
import mozilla.components.browser.menu.item.BrowserMenuImageText
import mozilla.components.browser.menu.item.BrowserMenuItemToolbar
import mozilla.components.browser.menu.item.WebExtensionPlaceholderMenuItem
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.storage.BookmarksStorage
import mozilla.components.feature.webcompat.reporter.WebCompatReporterFeature
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
Expand All @@ -36,17 +42,17 @@ import org.mozilla.fenix.theme.ThemeManager

/**
* Builds the toolbar object used with the 3-dot menu in the browser fragment.
* @param sessionManager Reference to the session manager that contains all tabs.
* @param store reference to the application's [BrowserStore].
* @param hasAccountProblem If true, there was a problem signing into the Firefox account.
* @param shouldReverseItems If true, reverse the menu items.
* @param onItemTapped Called when a menu item is tapped.
* @param lifecycleOwner View lifecycle owner used to determine when to cancel UI jobs.
* @param bookmarksStorage Used to check if a page is bookmarked.
*/
@Suppress("LargeClass", "LongParameterList")
@ExperimentalCoroutinesApi
class DefaultToolbarMenu(
private val context: Context,
private val sessionManager: SessionManager,
private val store: BrowserStore,
hasAccountProblem: Boolean = false,
shouldReverseItems: Boolean,
Expand All @@ -59,8 +65,7 @@ class DefaultToolbarMenu(
private var currentUrlIsBookmarked = false
private var isBookmarkedJob: Job? = null

/** Gets the current browser session */
private val session: Session? get() = sessionManager.selectedSession
private val selectedSession: TabSessionState? get() = store.state.selectedTab

override val menuBuilder by lazy {
WebExtensionBrowserMenuBuilder(
Expand All @@ -81,7 +86,7 @@ class DefaultToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_back),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.canGoBack ?: true
selectedSession?.content?.canGoBack ?: true
},
secondaryImageTintResource = ThemeManager.resolveAttribute(R.attr.disabled, context),
disableInSecondaryState = true,
Expand All @@ -95,7 +100,7 @@ class DefaultToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_forward),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.canGoForward ?: true
selectedSession?.content?.canGoForward ?: true
},
secondaryImageTintResource = ThemeManager.resolveAttribute(R.attr.disabled, context),
disableInSecondaryState = true,
Expand All @@ -109,15 +114,15 @@ class DefaultToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_refresh),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.loading == false
selectedSession?.content?.loading == false
},
secondaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_stop,
secondaryContentDescription = context.getString(R.string.browser_menu_stop),
secondaryImageTintResource = primaryTextColor(),
disableInSecondaryState = false,
longClickListener = { onItemTapped.invoke(ToolbarMenu.Item.Reload(bypassCache = true)) }
) {
if (session?.loading == true) {
if (selectedSession?.content?.loading == true) {
onItemTapped.invoke(ToolbarMenu.Item.Stop)
} else {
onItemTapped.invoke(ToolbarMenu.Item.Reload(bypassCache = false))
Expand Down Expand Up @@ -157,19 +162,19 @@ class DefaultToolbarMenu(

// Predicates that need to be repeatedly called as the session changes
private fun canAddToHomescreen(): Boolean =
session != null && isPinningSupported &&
selectedSession != null && isPinningSupported &&
!context.components.useCases.webAppUseCases.isInstallable()

private fun canInstall(): Boolean =
session != null && isPinningSupported &&
selectedSession != null && isPinningSupported &&
context.components.useCases.webAppUseCases.isInstallable()

private fun shouldShowOpenInApp(): Boolean = session?.let { session ->
private fun shouldShowOpenInApp(): Boolean = selectedSession?.let { session ->
val appLink = context.components.useCases.appLinksUseCases.appLinkRedirect
appLink(session.url).hasExternalApp()
appLink(session.content.url).hasExternalApp()
} ?: false

private fun shouldShowReaderAppearance(): Boolean = session?.let {
private fun shouldShowReaderAppearance(): Boolean = selectedSession?.let {
store.state.findTab(it.id)?.readerState?.active
} ?: false
// End of predicates //
Expand Down Expand Up @@ -234,7 +239,7 @@ class DefaultToolbarMenu(
imageResource = R.drawable.ic_desktop,
label = context.getString(R.string.browser_menu_desktop_site),
initialState = {
session?.desktopMode ?: false
selectedSession?.content?.desktopMode ?: false
}
) { checked ->
onItemTapped.invoke(ToolbarMenu.Item.RequestDesktop(checked))
Expand Down Expand Up @@ -353,44 +358,28 @@ class DefaultToolbarMenu(
}

@ColorRes
private fun primaryTextColor() = ThemeManager.resolveAttribute(R.attr.primaryText, context)

private var currentSessionObserver: Pair<Session, Session.Observer>? = null

private fun registerForIsBookmarkedUpdates() {
session?.let {
registerForUrlChanges(it)
}

val sessionManagerObserver = object : SessionManager.Observer {
override fun onSessionSelected(session: Session) {
// Unregister any old session observer before registering a new session observer
currentSessionObserver?.let {
it.first.unregister(it.second)
@VisibleForTesting
internal fun primaryTextColor() = ThemeManager.resolveAttribute(R.attr.primaryText, context)

@VisibleForTesting
internal fun registerForIsBookmarkedUpdates() {
store.flowScoped(lifecycleOwner) { flow ->
flow.mapNotNull { state -> state.selectedTab }
.ifAnyChanged { tab ->
arrayOf(
tab.id,
tab.content.url
)
}
.collect {
currentUrlIsBookmarked = false
updateCurrentUrlIsBookmarked(it.content.url)
}
currentUrlIsBookmarked = false
updateCurrentUrlIsBookmarked(session.url)
registerForUrlChanges(session)
}
}

sessionManager.register(sessionManagerObserver, lifecycleOwner)
}

private fun registerForUrlChanges(session: Session) {
val sessionObserver = object : Session.Observer {
override fun onUrlChanged(session: Session, url: String) {
currentUrlIsBookmarked = false
updateCurrentUrlIsBookmarked(url)
}
}

currentSessionObserver = Pair(session, sessionObserver)
updateCurrentUrlIsBookmarked(session.url)
session.register(sessionObserver, lifecycleOwner)
}

private fun updateCurrentUrlIsBookmarked(newUrl: String) {
@VisibleForTesting
internal fun updateCurrentUrlIsBookmarked(newUrl: String) {
isBookmarkedJob?.cancel()
isBookmarkedJob = lifecycleOwner.lifecycleScope.launch {
currentUrlIsBookmarked = bookmarksStorage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import mozilla.components.browser.menu.item.BrowserMenuImageSwitch
import mozilla.components.browser.menu.item.BrowserMenuImageText
import mozilla.components.browser.menu.item.BrowserMenuItemToolbar
import mozilla.components.browser.menu.item.SimpleBrowserMenuItem
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.store.BrowserStore
import org.mozilla.fenix.R
import org.mozilla.fenix.components.toolbar.ToolbarMenu
import org.mozilla.fenix.ext.components
Expand All @@ -29,14 +30,14 @@ import java.util.Locale

/**
* Builds the toolbar object used with the 3-dot menu in the custom tab browser fragment.
* @param sessionManager Reference to the session manager that contains all tabs.
* @param store reference to the application's [BrowserStore].
* @param sessionId ID of the open custom tab session.
* @param shouldReverseItems If true, reverse the menu items.
* @param onItemTapped Called when a menu item is tapped.
*/
class CustomTabToolbarMenu(
private val context: Context,
private val sessionManager: SessionManager,
private val store: BrowserStore,
private val sessionId: String?,
private val shouldReverseItems: Boolean,
private val onItemTapped: (ToolbarMenu.Item) -> Unit = {}
Expand All @@ -45,7 +46,7 @@ class CustomTabToolbarMenu(
override val menuBuilder by lazy { BrowserMenuBuilder(menuItems) }

/** Gets the current custom tab session */
private val session: Session? get() = sessionId?.let { sessionManager.findSessionById(it) }
private val session: TabSessionState? get() = sessionId?.let { store.state.findTab(it) }
private val appName = context.getString(R.string.app_name)

override val menuToolbar by lazy {
Expand All @@ -54,7 +55,7 @@ class CustomTabToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_back),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.canGoBack ?: true
session?.content?.canGoBack ?: true
},
secondaryImageTintResource = ThemeManager.resolveAttribute(
R.attr.disabled,
Expand All @@ -71,7 +72,7 @@ class CustomTabToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_forward),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.canGoForward ?: true
session?.content?.canGoForward ?: true
},
secondaryImageTintResource = ThemeManager.resolveAttribute(
R.attr.disabled,
Expand All @@ -88,15 +89,15 @@ class CustomTabToolbarMenu(
primaryContentDescription = context.getString(R.string.browser_menu_refresh),
primaryImageTintResource = primaryTextColor(),
isInPrimaryState = {
session?.loading == false
session?.content?.loading == false
},
secondaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_stop,
secondaryContentDescription = context.getString(R.string.browser_menu_stop),
secondaryImageTintResource = primaryTextColor(),
disableInSecondaryState = false,
longClickListener = { onItemTapped.invoke(ToolbarMenu.Item.Reload(bypassCache = true)) }
) {
if (session?.loading == true) {
if (session?.content?.loading == true) {
onItemTapped.invoke(ToolbarMenu.Item.Stop)
} else {
onItemTapped.invoke(ToolbarMenu.Item.Reload(bypassCache = false))
Expand All @@ -108,7 +109,7 @@ class CustomTabToolbarMenu(

private fun shouldShowOpenInApp(): Boolean = session?.let { session ->
val appLink = context.components.useCases.appLinksUseCases.appLinkRedirect
appLink(session.url).hasExternalApp()
appLink(session.content.url).hasExternalApp()
} ?: false

private val menuItems by lazy {
Expand All @@ -132,7 +133,7 @@ class CustomTabToolbarMenu(
private val desktopMode = BrowserMenuImageSwitch(
imageResource = R.drawable.ic_desktop,
label = context.getString(R.string.browser_menu_desktop_site),
initialState = { session?.desktopMode ?: false }
initialState = { session?.content?.desktopMode ?: false }
) { checked ->
onItemTapped.invoke(ToolbarMenu.Item.RequestDesktop(checked))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package org.mozilla.fenix.customtabs
import android.app.Activity
import androidx.appcompat.content.res.AppCompatResources.getDrawable
import mozilla.components.browser.session.SessionManager
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.browser.toolbar.display.DisplayToolbar
import mozilla.components.feature.customtabs.CustomTabsToolbarFeature
Expand All @@ -18,6 +19,7 @@ import org.mozilla.fenix.ext.settings

class CustomTabsIntegration(
sessionManager: SessionManager,
store: BrowserStore,
toolbar: BrowserToolbar,
sessionId: String,
activity: Activity,
Expand Down Expand Up @@ -74,7 +76,7 @@ class CustomTabsIntegration(
private val customTabToolbarMenu by lazy {
CustomTabToolbarMenu(
activity,
sessionManager,
store,
sessionId,
shouldReverseItems,
onItemTapped = onItemTapped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler
customTabsIntegration.set(
feature = CustomTabsIntegration(
sessionManager = requireComponents.core.sessionManager,
store = requireComponents.core.store,
toolbar = toolbar,
sessionId = customTabSessionId,
activity = activity,
Expand Down

0 comments on commit d1345e8

Please sign in to comment.