diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 5fa48ebd3629..3e8355a63244 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/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 ) 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 bfef18fa184a..8f56d67a550a 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 @@ -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 } 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 aec201d844a4..85f55cb5429b 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 @@ -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,34 @@ class DefaultBrowserToolbarMenuController( metrics.track(Event.SetDefaultBrowserToolbarMenuClicked) activity.openSetDefaultBrowserOption() } + is ToolbarMenu.Item.RemoveFromTopSites -> { + scope.launch { + val context = swipeRefresh.context + 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( + context.getString(R.string.snackbar_top_site_removed) + ) + .show() + } + } } } @@ -402,6 +432,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)) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index 08660015c319..78fa0a318baf 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/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 { 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 0630e8fe2ad7..c64d04b5aeea 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 @@ -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 pinned. * @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) } 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 8477905b998b..885e9e0d9118 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 @@ -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() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a35d620311ff..4152d1e191e9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1816,6 +1816,8 @@ Are you sure you want to delete this bookmark? Add to top sites + + Remove from top sites Verified By: %1$s diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index a5fd88abea89..4054f5ef76c3 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/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 @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 @@ -623,7 +649,7 @@ class DefaultBrowserToolbarMenuControllerTest { activity: HomeActivity = this.activity, customTabSessionId: String? = null, findInPageLauncher: () -> Unit = { }, - bookmarkTapped: (String, String) -> Unit = { _, _ -> } + bookmarkTapped: (String, String) -> Unit = { _, _ -> }, ) = DefaultBrowserToolbarMenuController( store = store, activity = activity, @@ -641,6 +667,7 @@ class DefaultBrowserToolbarMenuControllerTest { readerModeController = readerModeController, sessionFeature = sessionFeatureWrapper, topSitesStorage = topSitesStorage, + pinnedSiteStorage = pinnedSiteStorage, browserStore = browserStore ).apply { ioScope = scope diff --git a/app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt b/app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt index c2dd37d9ce1d..fead461ceeb3 100644 --- a/app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt +++ b/app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt @@ -6,17 +6,23 @@ package org.mozilla.fenix.toolbar import android.content.Context import android.net.Uri +import androidx.lifecycle.LifecycleCoroutineScope import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.lifecycleScope +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.spyk import io.mockk.unmockkStatic import kotlinx.coroutines.test.TestCoroutineDispatcher +import kotlinx.coroutines.test.TestCoroutineScope 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.feature.top.sites.TopSite import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After import org.junit.Assert.assertEquals @@ -35,6 +41,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 +59,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 +84,7 @@ class DefaultToolbarMenuTest { hasAccountProblem = false, onItemTapped = { }, lifecycleOwner = lifecycleOwner, + pinnedSiteStorage = pinnedSiteStorage, bookmarksStorage = bookmarksStorage, isPinningSupported = false ) @@ -146,4 +155,19 @@ class DefaultToolbarMenuTest { assertEquals(settingsItem, lastItem) } + + @Test + fun `WHEN the current url is pinned THEN addRemove top site item should be in secondary state`() { + val topSiteUrl = "https://firefox.com" + val topSite: TopSite = mockk() + every { topSite.url } returns topSiteUrl + every { context.settings().shouldUseBottomToolbar } returns false + every { lifecycleOwner.lifecycleScope } returns (TestCoroutineScope() as LifecycleCoroutineScope) + coEvery { pinnedSiteStorage.getPinnedSites() } returns listOf(topSite) + + createMenu() + + toolbarMenu.updateIsCurrentUrlPinned(topSiteUrl) + assertEquals(false, toolbarMenu.addRemoveTopSitesItem.isInPrimaryState()) + } }