From 761d6babb93dfa1edf62cd0b0e99565ebd3057f7 Mon Sep 17 00:00:00 2001 From: Elise Richards Date: Mon, 12 Apr 2021 16:01:33 -0500 Subject: [PATCH] For #18806: Show sync tabs or sync account sign in based on feature flag (#18859) * Show sync tabs or sync account sign in based on feature flag. Sign in navigates to account settings on click. * lint and nits --- .../mozilla/fenix/components/metrics/Event.kt | 2 +- .../toolbar/BrowserToolbarMenuController.kt | 16 ++++++---- .../components/toolbar/DefaultToolbarMenu.kt | 32 +++++++++++++------ .../fenix/components/toolbar/ToolbarMenu.kt | 3 +- app/src/main/res/navigation/nav_graph.xml | 3 ++ 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index 67c46cd66..860a3637e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -582,7 +582,7 @@ sealed class Event { NEW_PRIVATE_TAB, SHARE, BACK, FORWARD, RELOAD, STOP, OPEN_IN_FENIX, SAVE_TO_COLLECTION, ADD_TO_TOP_SITES, ADD_TO_HOMESCREEN, QUIT, READER_MODE_ON, READER_MODE_OFF, OPEN_IN_APP, BOOKMARK, READER_MODE_APPEARANCE, ADDONS_MANAGER, - BOOKMARKS, HISTORY, SYNC_TABS, DOWNLOADS, SET_DEFAULT_BROWSER + BOOKMARKS, HISTORY, SYNC_TABS, DOWNLOADS, SET_DEFAULT_BROWSER, SYNC_ACCOUNT } override val extras: Map? diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index cacbb2758..da48bb067 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -92,7 +92,7 @@ class DefaultBrowserToolbarMenuController( Do exhaustive when (item) { // TODO: These can be removed for https://github.com/mozilla-mobile/fenix/issues/17870 // todo === Start === - is ToolbarMenu.Item.InstallToHomeScreen -> { + is ToolbarMenu.Item.InstallPwaToHomeScreen -> { settings.installPwaOpened = true MainScope().launch { with(activity.components.useCases.webAppUseCases) { @@ -224,6 +224,12 @@ class DefaultBrowserToolbarMenuController( BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment() ) } + is ToolbarMenu.Item.SyncAccount -> browserAnimator.captureEngineViewAndDrawStatically { + navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment() + ) + } is ToolbarMenu.Item.RequestDesktop -> { currentSession?.let { sessionUseCases.requestDesktopSite.invoke( @@ -359,15 +365,12 @@ class DefaultBrowserToolbarMenuController( @Suppress("ComplexMethod") private fun trackToolbarItemInteraction(item: ToolbarMenu.Item) { val eventItem = when (item) { - // TODO: These can be removed for https://github.com/mozilla-mobile/fenix/issues/17870 - // todo === Start === is ToolbarMenu.Item.OpenInFenix -> Event.BrowserMenuItemTapped.Item.OPEN_IN_FENIX - is ToolbarMenu.Item.InstallToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN + is ToolbarMenu.Item.InstallPwaToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN is ToolbarMenu.Item.Quit -> Event.BrowserMenuItemTapped.Item.QUIT + is ToolbarMenu.Item.OpenInApp -> Event.BrowserMenuItemTapped.Item.OPEN_IN_APP is ToolbarMenu.Item.CustomizeReaderView -> Event.BrowserMenuItemTapped.Item.READER_MODE_APPEARANCE - is ToolbarMenu.Item.OpenInApp -> Event.BrowserMenuItemTapped.Item.OPEN_IN_APP - // todo === End === is ToolbarMenu.Item.Back -> Event.BrowserMenuItemTapped.Item.BACK is ToolbarMenu.Item.Forward -> Event.BrowserMenuItemTapped.Item.FORWARD is ToolbarMenu.Item.Reload -> Event.BrowserMenuItemTapped.Item.RELOAD @@ -385,6 +388,7 @@ class DefaultBrowserToolbarMenuController( is ToolbarMenu.Item.AddToTopSites -> Event.BrowserMenuItemTapped.Item.ADD_TO_TOP_SITES is ToolbarMenu.Item.AddToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN is ToolbarMenu.Item.SyncedTabs -> Event.BrowserMenuItemTapped.Item.SYNC_TABS + is ToolbarMenu.Item.SyncAccount -> Event.BrowserMenuItemTapped.Item.SYNC_ACCOUNT is ToolbarMenu.Item.Bookmark -> Event.BrowserMenuItemTapped.Item.BOOKMARK is ToolbarMenu.Item.AddonsManager -> Event.BrowserMenuItemTapped.Item.ADDONS_MANAGER is ToolbarMenu.Item.Bookmarks -> Event.BrowserMenuItemTapped.Item.BOOKMARKS diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index 99c20bb75..d2a231670 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -35,6 +35,7 @@ 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.FeatureFlags +import org.mozilla.fenix.FeatureFlags.tabsTrayRewrite import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode @@ -216,7 +217,7 @@ open class DefaultToolbarMenu( !context.settings().installPwaOpened } ) { - onItemTapped.invoke(ToolbarMenu.Item.InstallToHomeScreen) + onItemTapped.invoke(ToolbarMenu.Item.InstallPwaToHomeScreen) } private val oldCoreMenuItems by lazy { @@ -417,13 +418,26 @@ open class DefaultToolbarMenu( context.getString(R.string.sync_menu_sign_in) } - val syncedTabs = BrowserMenuImageText( - syncItemTitle, - R.drawable.ic_synced_tabs, - primaryTextColor() - ) { - onItemTapped.invoke(ToolbarMenu.Item.SyncedTabs) - } + val syncTabsOrSignInItem = + if (tabsTrayRewrite) { + // If synced tabs are being shown in tabs tray, show sync sign in here. + BrowserMenuImageText( + syncItemTitle, + R.drawable.ic_synced_tabs, + primaryTextColor() + ) { + onItemTapped.invoke(ToolbarMenu.Item.SyncAccount) + } + } else { + // If synced tabs are not shown in tabs tray, they should be shown here. + BrowserMenuImageText( + context.getString(R.string.synced_tabs), + R.drawable.ic_synced_tabs, + primaryTextColor() + ) { + onItemTapped.invoke(ToolbarMenu.Item.SyncedTabs) + } + } val findInPageItem = BrowserMenuImageText( label = context.getString(R.string.browser_menu_find_in_page), @@ -548,7 +562,7 @@ open class DefaultToolbarMenu( historyItem, downloadsItem, extensionsItem, - syncedTabs, + syncTabsOrSignInItem, BrowserMenuDivider(), findInPageItem, desktopSiteItem, diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt index 0c8530335..7b547e7fe 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt @@ -20,9 +20,10 @@ interface ToolbarMenu { object OpenInFenix : Item() object SaveToCollection : Item() object AddToTopSites : Item() - object InstallToHomeScreen : Item() + object InstallPwaToHomeScreen : Item() object AddToHomeScreen : Item() object SyncedTabs : Item() + object SyncAccount : Item() object AddonsManager : Item() object Quit : Item() object OpenInApp : Item() diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index c982d10d8..4bb00e985 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -214,6 +214,9 @@ +