For #18867: remove "signed in as" string in three-dot menu (#19035)

* Remove signed in as string from sync menu item

* Nav to sync account settings on click

For #18806: navigate to settings account page or sign in on clicking menu item.

* Confirm account exists and retrieve item title

* Remove string

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt

@@ -225,11 +225,13 @@ class DefaultBrowserToolbarMenuController(
                     BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment()
                 )
             }
-            is ToolbarMenu.Item.SyncAccount -> browserAnimator.captureEngineViewAndDrawStatically {
-                navController.nav(
-                    R.id.browserFragment,
-                    BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment()
-                )
+            is ToolbarMenu.Item.SyncAccount -> {
+                browserAnimator.captureEngineViewAndDrawStatically {
+                    navController.nav(
+                        R.id.browserFragment,
+                        BrowserFragmentDirections.actionGlobalAccountSettingsFragment()
+                    )
+                }
             }
             is ToolbarMenu.Item.RequestDesktop -> {
                 currentSession?.let {

app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt

@@ -57,7 +57,7 @@ import org.mozilla.fenix.utils.BrowsersCache
  * @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")
+@Suppress("LargeClass", "LongParameterList", "TooManyFunctions")
 @ExperimentalCoroutinesApi
 open class DefaultToolbarMenu(
     private val context: Context,
@@ -78,8 +78,7 @@ open class DefaultToolbarMenu(
     private val selectedSession: TabSessionState?
         get() = store.state.selectedTab
 
-    private val primaryTextColor =
-        ThemeManager.resolveAttribute(R.attr.primaryText, context)
+    private val accountManager = context.components.backgroundServices.accountManager
 
     override val menuBuilder by lazy {
         WebExtensionBrowserMenuBuilder(
@@ -91,7 +90,7 @@ open class DefaultToolbarMenu(
                 },
             endOfMenuAlwaysVisible = shouldUseBottomToolbar,
             store = store,
-            webExtIconTintColorResource = primaryTextColor,
+            webExtIconTintColorResource = primaryTextColor(),
             onAddonsManagerTapped = {
                 onItemTapped.invoke(ToolbarMenu.Item.AddonsManager)
             },
@@ -103,7 +102,7 @@ open class DefaultToolbarMenu(
         val back = BrowserMenuItemToolbar.TwoStateButton(
             primaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_back,
             primaryContentDescription = context.getString(R.string.browser_menu_back),
-            primaryImageTintResource = primaryTextColor,
+            primaryImageTintResource = primaryTextColor(),
             isInPrimaryState = {
                 selectedSession?.content?.canGoBack ?: true
             },
@@ -117,7 +116,7 @@ open class DefaultToolbarMenu(
         val forward = BrowserMenuItemToolbar.TwoStateButton(
             primaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_forward,
             primaryContentDescription = context.getString(R.string.browser_menu_forward),
-            primaryImageTintResource = primaryTextColor,
+            primaryImageTintResource = primaryTextColor(),
             isInPrimaryState = {
                 selectedSession?.content?.canGoForward ?: true
             },
@@ -131,13 +130,13 @@ open class DefaultToolbarMenu(
         val refresh = BrowserMenuItemToolbar.TwoStateButton(
             primaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_refresh,
             primaryContentDescription = context.getString(R.string.browser_menu_refresh),
-            primaryImageTintResource = primaryTextColor,
+            primaryImageTintResource = primaryTextColor(),
             isInPrimaryState = {
                 selectedSession?.content?.loading == false
             },
             secondaryImageResource = mozilla.components.ui.icons.R.drawable.mozac_ic_stop,
             secondaryContentDescription = context.getString(R.string.browser_menu_stop),
-            secondaryImageTintResource = primaryTextColor,
+            secondaryImageTintResource = primaryTextColor(),
             disableInSecondaryState = false,
             longClickListener = { onItemTapped.invoke(ToolbarMenu.Item.Reload(bypassCache = true)) }
         ) {
@@ -151,7 +150,7 @@ open class DefaultToolbarMenu(
         val share = BrowserMenuItemToolbar.Button(
             imageResource = R.drawable.ic_share_filled,
             contentDescription = context.getString(R.string.browser_menu_share),
-            iconTintColorResource = primaryTextColor,
+            iconTintColorResource = primaryTextColor(),
             listener = {
                 onItemTapped.invoke(ToolbarMenu.Item.Share)
             }
@@ -165,14 +164,14 @@ open class DefaultToolbarMenu(
             val bookmark = BrowserMenuItemToolbar.TwoStateButton(
                 primaryImageResource = R.drawable.ic_bookmark_filled,
                 primaryContentDescription = context.getString(R.string.browser_menu_edit_bookmark),
-                primaryImageTintResource = primaryTextColor,
+                primaryImageTintResource = primaryTextColor(),
                 // TwoStateButton.isInPrimaryState must be synchronous, and checking bookmark state is
                 // relatively slow. The best we can do here is periodically compute and cache a new "is
                 // bookmarked" state, and use that whenever the menu has been opened.
                 isInPrimaryState = { isCurrentUrlBookmarked },
                 secondaryImageResource = R.drawable.ic_bookmark_outline,
                 secondaryContentDescription = context.getString(R.string.browser_menu_bookmark),
-                secondaryImageTintResource = primaryTextColor,
+                secondaryImageTintResource = primaryTextColor(),
                 disableInSecondaryState = false
             ) {
                 handleBookmarkItemTapped()
@@ -208,7 +207,7 @@ open class DefaultToolbarMenu(
     val installToHomescreen = BrowserMenuHighlightableItem(
         label = context.getString(R.string.browser_menu_install_on_homescreen),
         startImageResource = R.drawable.ic_add_to_homescreen,
-        iconTintColorResource = primaryTextColor,
+        iconTintColorResource = primaryTextColor(),
         highlight = BrowserMenuHighlight.LowPriority(
             label = context.getString(R.string.browser_menu_install_on_homescreen),
             notificationTint = getColor(context, R.color.whats_new_notification_color)
@@ -226,10 +225,10 @@ open class DefaultToolbarMenu(
             startImageResource = R.drawable.ic_settings,
             iconTintColorResource = if (hasAccountProblem)
                 ThemeManager.resolveAttribute(R.attr.syncDisconnected, context) else
-                primaryTextColor,
+                primaryTextColor(),
             textColorResource = if (hasAccountProblem)
                 ThemeManager.resolveAttribute(R.attr.primaryText, context) else
-                primaryTextColor,
+                primaryTextColor(),
             highlight = BrowserMenuHighlight.HighPriority(
                 endImageResource = R.drawable.ic_sync_disconnected,
                 backgroundTint = context.getColorFromAttr(R.attr.syncDisconnectedBackground),
@@ -253,7 +252,7 @@ open class DefaultToolbarMenu(
         val addToTopSites = BrowserMenuImageText(
             label = context.getString(R.string.browser_menu_add_to_top_sites),
             imageResource = R.drawable.ic_top_sites,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.AddToTopSites)
         }
@@ -261,7 +260,7 @@ open class DefaultToolbarMenu(
         val addToHomescreen = BrowserMenuImageText(
             label = context.getString(R.string.browser_menu_add_to_homescreen),
             imageResource = R.drawable.ic_add_to_homescreen,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.AddToHomeScreen)
         }
@@ -269,7 +268,7 @@ open class DefaultToolbarMenu(
         val syncedTabs = BrowserMenuImageText(
             label = context.getString(R.string.synced_tabs),
             imageResource = R.drawable.ic_synced_tabs,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.SyncedTabs)
         }
@@ -277,7 +276,7 @@ open class DefaultToolbarMenu(
         val findInPage = BrowserMenuImageText(
             label = context.getString(R.string.browser_menu_find_in_page),
             imageResource = R.drawable.mozac_ic_search,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.FindInPage)
         }
@@ -289,7 +288,7 @@ open class DefaultToolbarMenu(
         val saveToCollection = BrowserMenuImageText(
             label = context.getString(R.string.browser_menu_save_to_collection_2),
             imageResource = R.drawable.ic_tab_collection,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.SaveToCollection)
         }
@@ -297,7 +296,7 @@ open class DefaultToolbarMenu(
         val deleteDataOnQuit = BrowserMenuImageText(
             label = context.getString(R.string.delete_browsing_data_on_quit_action),
             imageResource = R.drawable.ic_exit,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.Quit)
         }
@@ -305,7 +304,7 @@ open class DefaultToolbarMenu(
         val readerAppearance = BrowserMenuImageText(
             label = context.getString(R.string.browser_menu_read_appearance),
             imageResource = R.drawable.ic_readermode_appearance,
-            iconTintColorResource = primaryTextColor
+            iconTintColorResource = primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.CustomizeReaderView)
         }
@@ -313,7 +312,7 @@ open class DefaultToolbarMenu(
         val openInApp = BrowserMenuHighlightableItem(
             label = context.getString(R.string.browser_menu_open_app_link),
             startImageResource = R.drawable.ic_open_in_app,
-            iconTintColorResource = primaryTextColor,
+            iconTintColorResource = primaryTextColor(),
             highlight = BrowserMenuHighlight.LowPriority(
                 label = context.getString(R.string.browser_menu_open_app_link),
                 notificationTint = getColor(context, R.color.whats_new_notification_color)
@@ -326,7 +325,7 @@ open class DefaultToolbarMenu(
         val historyItem = BrowserMenuImageText(
             context.getString(R.string.library_history),
             R.drawable.ic_history,
-            primaryTextColor
+            primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.History)
         }
@@ -334,7 +333,7 @@ open class DefaultToolbarMenu(
         val bookmarksItem = BrowserMenuImageText(
             context.getString(R.string.library_bookmarks),
             R.drawable.ic_bookmark_filled,
-            primaryTextColor
+            primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.Bookmarks)
         }
@@ -342,7 +341,7 @@ open class DefaultToolbarMenu(
         val downloadsItem = BrowserMenuImageText(
             context.getString(R.string.library_downloads),
             R.drawable.ic_download,
-            primaryTextColor
+            primaryTextColor()
         ) {
             onItemTapped.invoke(ToolbarMenu.Item.Downloads)
         }
@@ -410,35 +409,6 @@ open class DefaultToolbarMenu(
         id = WebExtensionPlaceholderMenuItem.MAIN_EXTENSIONS_MENU_ID
     )
 
-    val accountManager = context.components.backgroundServices.accountManager
-    val account = accountManager.authenticatedAccount()
-    val syncItemTitle = if (account != null && accountManager.accountProfile()?.email != null) {
-        context.getString(R.string.sync_signed_as, accountManager.accountProfile()?.email)
-    } else {
-        context.getString(R.string.sync_menu_sign_in)
-    }
-
-    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),
         imageResource = R.drawable.mozac_ic_search,
@@ -512,10 +482,10 @@ open class DefaultToolbarMenu(
         startImageResource = R.drawable.ic_settings,
         iconTintColorResource = if (hasAccountProblem)
             ThemeManager.resolveAttribute(R.attr.syncDisconnected, context) else
-            primaryTextColor,
+            primaryTextColor(),
         textColorResource = if (hasAccountProblem)
             ThemeManager.resolveAttribute(R.attr.primaryText, context) else
-            primaryTextColor,
+            primaryTextColor(),
         highlight = BrowserMenuHighlight.HighPriority(
             endImageResource = R.drawable.ic_sync_disconnected,
             backgroundTint = context.getColorFromAttr(R.attr.syncDisconnectedBackground),
@@ -546,11 +516,38 @@ open class DefaultToolbarMenu(
     val deleteDataOnQuit = BrowserMenuImageText(
         label = context.getString(R.string.delete_browsing_data_on_quit_action),
         imageResource = R.drawable.ic_exit,
-        iconTintColorResource = primaryTextColor
+        iconTintColorResource = primaryTextColor()
     ) {
         onItemTapped.invoke(ToolbarMenu.Item.Quit)
     }
 
+    val syncedTabsItem = BrowserMenuImageText(
+        context.getString(R.string.synced_tabs),
+        R.drawable.ic_synced_tabs,
+        primaryTextColor()
+    ) {
+        onItemTapped.invoke(ToolbarMenu.Item.SyncedTabs)
+    }
+
+    private fun getSyncItemTitle(): String {
+        val authenticatedAccount = accountManager.authenticatedAccount() != null
+        val email = accountManager.accountProfile()?.email
+
+        return if (authenticatedAccount && email != null) {
+            email
+        } else {
+            context.getString(R.string.sync_menu_sign_in)
+        }
+    }
+
+    val syncMenuItem = BrowserMenuImageText(
+        getSyncItemTitle(),
+        R.drawable.ic_synced_tabs,
+        primaryTextColor()
+    ) {
+        onItemTapped.invoke(ToolbarMenu.Item.SyncAccount)
+    }
+
     @VisibleForTesting(otherwise = PRIVATE)
     val newCoreMenuItems by lazy {
         val menuItems =
@@ -562,7 +559,7 @@ open class DefaultToolbarMenu(
                 historyItem,
                 downloadsItem,
                 extensionsItem,
-                syncTabsOrSignInItem,
+                if (tabsTrayRewrite) syncMenuItem else syncedTabsItem,
                 BrowserMenuDivider(),
                 findInPageItem,
                 desktopSiteItem,
@@ -623,6 +620,7 @@ open class DefaultToolbarMenu(
                 .any { it.url == newUrl }
         }
     }
+
     private fun getSetDefaultBrowserItem(): BrowserMenuImageText? {
         val experiments = context.components.analytics.experiments
         val browsers = BrowsersCache.all(context)

app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt

@@ -63,6 +63,7 @@ class HomeMenu(
         context.getColorFromAttr(R.attr.syncDisconnectedBackground)
 
     private val shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar
+    private val accountManager = context.components.backgroundServices.accountManager
 
     // 'Reconnect' and 'Quit' items aren't needed most of the time, so we'll only create the if necessary.
     private val reconnectToSyncItem by lazy {
@@ -91,6 +92,17 @@ class HomeMenu(
         }
     }
 
+    private fun getSyncItemTitle(): String {
+        val authenticatedAccount = accountManager.authenticatedAccount() != null
+        val email = accountManager.accountProfile()?.email
+
+        return if (authenticatedAccount && email != null) {
+            email
+        } else {
+            context.getString(R.string.sync_menu_sign_in)
+        }
+    }
+
     private val oldCoreMenuItems by lazy {
         val whatsNewItem = BrowserMenuHighlightableItem(
             context.getString(R.string.browser_menu_whats_new),
@@ -157,16 +169,8 @@ class HomeMenu(
             onItemTapped.invoke(Item.Settings)
         }
 
-        val accountManager = context.components.backgroundServices.accountManager
-        val account = accountManager.authenticatedAccount()
-        val syncItemTitle = if (account != null && accountManager.accountProfile()?.email != null) {
-            context.getString(R.string.sync_signed_as, accountManager.accountProfile()?.email)
-        } else {
-            context.getString(R.string.sync_menu_sign_in)
-        }
-
         val syncedTabsItem = BrowserMenuImageText(
-            syncItemTitle,
+            getSyncItemTitle(),
             R.drawable.ic_synced_tabs,
             primaryTextColor
         ) {

app/src/main/res/navigation/nav_graph.xml

@@ -214,9 +214,6 @@
         <action
             android:id="@+id/action_browserFragment_to_settingsFragment"
             app:destination="@id/settingsFragment" />
-        <action
-            android:id="@+id/action_browserFragment_to_syncAccountSettingsFragment"
-            app:destination="@id/accountSettingsFragment" />
         <action
             android:id="@+id/action_browserFragment_to_createShortcutFragment"
             app:destination="@id/createShortcutFragment" />

app/src/main/res/values/strings.xml

@@ -960,8 +960,6 @@
     <string name="share_link_all_apps_subheader">All actions</string>
     <!-- Sub-header in the dialog to share a link to an app from the most-recent sorted list -->
     <string name="share_link_recent_apps_subheader">Recently used</string>
-    <!-- An string shown when an account is signed in where %1$s is a placeholder for the email-->
-    <string name="sync_signed_as">Signed in as %1$s</string>
     <!-- An option from the three dot menu to into sync -->
     <string name="sync_menu_sign_in">Sign in to sync</string>
     <!-- An option from the share dialog to sign into sync -->