For #8108 - Add BrowserToolbar option to remove url from TopSites

app/metrics.yaml

@@ -127,9 +127,9 @@ events:
           bookmarks, desktop_view_off, desktop_view_on, downloads,
           find_in_page, forward, help, history, library, new_private_tab,
           new_tab, open_in_app, open_in_fenix, quit, reader_mode_appearance,
-          reader_mode_off, reader_mode_on, reload, save_to_collection,
-          set_default_browser, settings, share, stop, sync_account, and
-          sync_tabs.
+          reader_mode_off, reader_mode_on, reload, remove_from_top_sites,
+          save_to_collection, set_default_browser, settings, share, stop,
+          sync_account, and sync_tabs.
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/1024
       - https://github.com/mozilla-mobile/fenix/issues/19923

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -370,6 +370,7 @@ abstract class BaseBrowserFragment :
             scope = viewLifecycleOwner.lifecycleScope,
             tabCollectionStorage = requireComponents.core.tabCollectionStorage,
             topSitesStorage = requireComponents.core.topSitesStorage,
+            pinnedSiteStorage = requireComponents.core.pinnedSiteStorage,
             browserStore = store
         )
 

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -632,7 +632,7 @@ sealed class Event {
         enum class Item {
             SETTINGS, HELP, DESKTOP_VIEW_ON, DESKTOP_VIEW_OFF, FIND_IN_PAGE, NEW_TAB,
             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,
+            SAVE_TO_COLLECTION, ADD_TO_TOP_SITES, REMOVE_FROM_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, SYNC_ACCOUNT
         }

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

@@ -24,6 +24,7 @@ import mozilla.components.concept.engine.EngineSession.LoadUrlFlags
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.feature.session.SessionFeature
 import mozilla.components.feature.top.sites.DefaultTopSitesStorage
+import mozilla.components.feature.top.sites.PinnedSiteStorage
 import mozilla.components.feature.top.sites.TopSite
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import org.mozilla.fenix.HomeActivity
@@ -72,6 +73,7 @@ class DefaultBrowserToolbarMenuController(
     private val scope: CoroutineScope,
     private val tabCollectionStorage: TabCollectionStorage,
     private val topSitesStorage: DefaultTopSitesStorage,
+    private val pinnedSiteStorage: PinnedSiteStorage,
     private val browserStore: BrowserStore
 ) : BrowserToolbarMenuController {
 
@@ -355,6 +357,33 @@ class DefaultBrowserToolbarMenuController(
                 metrics.track(Event.SetDefaultBrowserToolbarMenuClicked)
                 activity.openSetDefaultBrowserOption()
             }
+            is ToolbarMenu.Item.RemoveFromTopSites -> {
+                scope.launch {
+                    val removedTopSite: TopSite? =
+                        pinnedSiteStorage
+                            .getPinnedSites()
+                            .find { it.url == currentSession?.content?.url }
+                    if (removedTopSite != null) {
+                        ioScope.launch {
+                            currentSession?.let {
+                                with(activity.components.useCases.topSitesUseCase) {
+                                    removeTopSites(removedTopSite)
+                                }
+                            }
+                        }.join()
+                    }
+
+                    FenixSnackbar.make(
+                        view = swipeRefresh,
+                        duration = Snackbar.LENGTH_SHORT,
+                        isDisplayedWithBrowserToolbar = true
+                    )
+                        .setText(
+                            swipeRefresh.context.getString(R.string.snackbar_top_site_removed)
+                        )
+                        .show()
+                }
+            }
         }
     }
 
@@ -402,6 +431,7 @@ class DefaultBrowserToolbarMenuController(
             is ToolbarMenu.Item.Downloads -> Event.BrowserMenuItemTapped.Item.DOWNLOADS
             is ToolbarMenu.Item.NewTab -> Event.BrowserMenuItemTapped.Item.NEW_TAB
             is ToolbarMenu.Item.SetDefaultBrowser -> Event.BrowserMenuItemTapped.Item.SET_DEFAULT_BROWSER
+            is ToolbarMenu.Item.RemoveFromTopSites -> Event.BrowserMenuItemTapped.Item.REMOVE_FROM_TOP_SITES
         }
 
         metrics.track(Event.BrowserMenuItemTapped(eventItem))

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

@@ -154,6 +154,7 @@ class BrowserToolbarView(
                     },
                     lifecycleOwner = lifecycleOwner,
                     bookmarksStorage = bookmarkStorage,
+                    pinnedSiteStorage = components.core.pinnedSiteStorage,
                     isPinningSupported = isPinningSupported
                 )
                 view.display.setMenuDismissAction {

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

@@ -23,12 +23,14 @@ import mozilla.components.browser.menu.item.BrowserMenuImageSwitch
 import mozilla.components.browser.menu.item.BrowserMenuImageText
 import mozilla.components.browser.menu.item.BrowserMenuImageTextCheckboxButton
 import mozilla.components.browser.menu.item.BrowserMenuItemToolbar
+import mozilla.components.browser.menu.item.TwoStateBrowserMenuImageText
 import mozilla.components.browser.menu.item.WebExtensionPlaceholderMenuItem
 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.top.sites.PinnedSiteStorage
 import mozilla.components.feature.webcompat.reporter.WebCompatReporterFeature
 import mozilla.components.lib.state.ext.flowScoped
 import mozilla.components.support.ktx.android.content.getColorFromAttr
@@ -48,6 +50,7 @@ import org.mozilla.fenix.utils.BrowsersCache
  * @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 pinnedSiteStorage Used to check if the current url is a pinned site.
  * @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.
@@ -60,9 +63,11 @@ open class DefaultToolbarMenu(
     private val onItemTapped: (ToolbarMenu.Item) -> Unit = {},
     private val lifecycleOwner: LifecycleOwner,
     private val bookmarksStorage: BookmarksStorage,
+    private val pinnedSiteStorage: PinnedSiteStorage,
     val isPinningSupported: Boolean
 ) : ToolbarMenu {
 
+    private var isCurrentUrlPinned = false
     private var isCurrentUrlBookmarked = false
     private var isBookmarkedJob: Job? = null
 
@@ -271,13 +276,23 @@ open class DefaultToolbarMenu(
         onItemTapped.invoke(ToolbarMenu.Item.AddToHomeScreen)
     }
 
-    val addToTopSitesItem = BrowserMenuImageText(
-        label = context.getString(R.string.browser_menu_add_to_top_sites),
-        imageResource = R.drawable.ic_top_sites,
-        iconTintColorResource = primaryTextColor()
-    ) {
-        onItemTapped.invoke(ToolbarMenu.Item.AddToTopSites)
-    }
+    val addRemoveTopSitesItem = TwoStateBrowserMenuImageText(
+        primaryLabel = context.getString(R.string.browser_menu_add_to_top_sites),
+        secondaryLabel = context.getString(R.string.browser_menu_remove_from_top_sites),
+        primaryStateIconResource = R.drawable.ic_top_sites,
+        secondaryStateIconResource = R.drawable.ic_top_sites,
+        iconTintColorResource = primaryTextColor(),
+        isInPrimaryState = { !isCurrentUrlPinned },
+        isInSecondaryState = { isCurrentUrlPinned },
+        primaryStateAction = {
+            isCurrentUrlPinned = true
+            onItemTapped.invoke(ToolbarMenu.Item.AddToTopSites)
+        },
+        secondaryStateAction = {
+            isCurrentUrlPinned = false
+            onItemTapped.invoke(ToolbarMenu.Item.RemoveFromTopSites)
+        }
+    )
 
     val saveToCollectionItem = BrowserMenuImageText(
         label = context.getString(R.string.browser_menu_save_to_collection_2),
@@ -367,7 +382,7 @@ open class DefaultToolbarMenu(
                 BrowserMenuDivider(),
                 addToHomeScreenItem.apply { visible = ::canAddToHomescreen },
                 installToHomescreen.apply { visible = ::canInstall },
-                addToTopSitesItem,
+                addRemoveTopSitesItem,
                 saveToCollectionItem,
                 BrowserMenuDivider(),
                 settingsItem,
@@ -392,6 +407,15 @@ open class DefaultToolbarMenu(
     @VisibleForTesting
     internal fun menuItemButtonTintColor() = ThemeManager.resolveAttribute(R.attr.menuItemButtonTintColor, context)
 
+    @VisibleForTesting
+    internal fun updateIsCurrentUrlPinned(currentUrl: String) {
+        lifecycleOwner.lifecycleScope.launch {
+            isCurrentUrlPinned = pinnedSiteStorage
+                .getPinnedSites()
+                .find { it.url == currentUrl } != null
+        }
+    }
+
     @VisibleForTesting
     internal fun registerForIsBookmarkedUpdates() {
         store.flowScoped(lifecycleOwner) { flow ->
@@ -403,6 +427,9 @@ open class DefaultToolbarMenu(
                     )
                 }
                 .collect {
+                    isCurrentUrlPinned = false
+                    updateIsCurrentUrlPinned(it.content.url)
+
                     isCurrentUrlBookmarked = false
                     updateCurrentUrlIsBookmarked(it.content.url)
                 }

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

@@ -21,6 +21,7 @@ interface ToolbarMenu {
         object OpenInFenix : Item()
         object SaveToCollection : Item()
         object AddToTopSites : Item()
+        object RemoveFromTopSites : Item()
         object InstallPwaToHomeScreen : Item()
         object AddToHomeScreen : Item()
         data class SyncAccount(val accountState: AccountState) : Item()

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

@@ -1172,7 +1172,7 @@
     <!-- Text shown in snackbar to undo deleting a tab, top site or collection -->
     <string name="snackbar_deleted_undo">UNDO</string>
     <!-- Text shown in snackbar when user removes a top site -->
-    <string name="snackbar_top_site_removed" moz:removedIn="96" tools:ignore="UnusedResources">Site removed</string>
+    <string name="snackbar_top_site_removed">Site removed</string>
     <!-- Text for action to undo deleting a tab or collection shown in a11y dialog -->
     <string name="a11y_dialog_deleted_undo" moz:removedIn="96" tools:ignore="UnusedResources">Undo</string>
     <!-- Text for action to confirm deleting a tab or collection shown in a11y dialog -->
@@ -1808,6 +1808,8 @@
     <string name="bookmark_deletion_confirmation">Are you sure you want to delete this bookmark?</string>
     <!-- Browser menu button that adds a top site to the home fragment -->
     <string name="browser_menu_add_to_top_sites">Add to top sites</string>
+    <!-- Browser menu button that removes a top site from the home fragment -->
+    <string name="browser_menu_remove_from_top_sites">Remove from top sites</string>
     <!-- text shown before the issuer name to indicate who its verified by, parameter is the name of
      the certificate authority that verified the ticket-->
     <string name="certificate_info_verified_by">Verified By: %1$s </string>

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt

@@ -9,6 +9,7 @@ import androidx.navigation.NavController
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
 import io.mockk.MockKAnnotations
 import io.mockk.Runs
+import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.impl.annotations.MockK
 import io.mockk.impl.annotations.RelaxedMockK
@@ -39,6 +40,8 @@ import mozilla.components.feature.session.SessionUseCases
 import mozilla.components.feature.tab.collections.TabCollection
 import mozilla.components.feature.tabs.CustomTabsUseCases
 import mozilla.components.feature.top.sites.DefaultTopSitesStorage
+import mozilla.components.feature.top.sites.PinnedSiteStorage
+import mozilla.components.feature.top.sites.TopSite
 import mozilla.components.feature.top.sites.TopSitesUseCases
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.test.ext.joinBlocking
@@ -92,6 +95,7 @@ class DefaultBrowserToolbarMenuControllerTest {
     @MockK private lateinit var sessionFeatureWrapper: ViewBoundFeatureWrapper<SessionFeature>
     @RelaxedMockK private lateinit var sessionFeature: SessionFeature
     @RelaxedMockK private lateinit var topSitesStorage: DefaultTopSitesStorage
+    @RelaxedMockK private lateinit var pinnedSiteStorage: PinnedSiteStorage
 
     private lateinit var browserStore: BrowserStore
     private lateinit var selectedTab: TabSessionState
@@ -421,6 +425,28 @@ class DefaultBrowserToolbarMenuControllerTest {
         verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.ADD_TO_TOP_SITES)) }
     }
 
+    @Test
+    fun `GIVEN a top site page is open WHEN Remove from top sites is pressed THEN show snackbar`() = runBlockingTest {
+        val snackbarMessage = "Site removed"
+        val item = ToolbarMenu.Item.RemoveFromTopSites
+        val removePinnedSiteUseCase: TopSitesUseCases.RemoveTopSiteUseCase =
+            mockk(relaxed = true)
+        val topSite: TopSite = mockk()
+        every { topSite.url } returns selectedTab.content.url
+        coEvery { pinnedSiteStorage.getPinnedSites() } returns listOf(topSite)
+        every { topSitesUseCase.removeTopSites } returns removePinnedSiteUseCase
+        every {
+            swipeRefreshLayout.context.getString(R.string.snackbar_top_site_removed)
+        } returns snackbarMessage
+
+        val controller = createController(scope = this, store = browserStore)
+        controller.handleToolbarItemInteraction(item)
+
+        verify { snackbar.setText(snackbarMessage) }
+        verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.REMOVE_FROM_TOP_SITES)) }
+        verify { removePinnedSiteUseCase.invoke(topSite) }
+    }
+
     @Test
     fun `WHEN addon extensions menu item is pressed THEN navigate to addons manager`() = runBlockingTest {
         val item = ToolbarMenu.Item.AddonsManager
@@ -641,6 +667,7 @@ class DefaultBrowserToolbarMenuControllerTest {
         readerModeController = readerModeController,
         sessionFeature = sessionFeatureWrapper,
         topSitesStorage = topSitesStorage,
+        pinnedSiteStorage = pinnedSiteStorage,
         browserStore = browserStore
     ).apply {
         ioScope = scope

app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt

@@ -17,6 +17,7 @@ import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.storage.BookmarksStorage
+import mozilla.components.feature.top.sites.PinnedSiteStorage
 import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -35,6 +36,7 @@ class DefaultToolbarMenuTest {
     private lateinit var toolbarMenu: DefaultToolbarMenu
     private lateinit var context: Context
     private lateinit var bookmarksStorage: BookmarksStorage
+    private lateinit var pinnedSiteStorage: PinnedSiteStorage
 
     private val testDispatcher = TestCoroutineDispatcher()
 
@@ -52,6 +54,7 @@ class DefaultToolbarMenuTest {
         every { context.theme } returns mockk(relaxed = true)
 
         bookmarksStorage = mockk(relaxed = true)
+        pinnedSiteStorage = mockk(relaxed = true)
         store = BrowserStore(
             BrowserState(
                 tabs = listOf(
@@ -76,6 +79,7 @@ class DefaultToolbarMenuTest {
                 hasAccountProblem = false,
                 onItemTapped = { },
                 lifecycleOwner = lifecycleOwner,
+                pinnedSiteStorage = pinnedSiteStorage,
                 bookmarksStorage = bookmarksStorage,
                 isPinningSupported = false
             )