diff --git a/app/metrics.yaml b/app/metrics.yaml index 904a3635b7..c20d0f0988 100644 --- a/app/metrics.yaml +++ b/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 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 5fa48ebd36..3e8355a632 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 bfef18fa18..8f56d67a55 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 aec201d844..48f67cefd7 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,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)) 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 08660015c3..78fa0a318b 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 0630e8fe2a..a5e3bf274b 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 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) } 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 8477905b99..885e9e0d91 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 7fae7d7cc3..3027300869 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1172,7 +1172,7 @@ UNDO - Site removed + Site removed Undo @@ -1808,6 +1808,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 a5fd88abea..b40d7ef65e 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 @@ -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 c2dd37d9ce..90953e2f9a 100644 --- a/app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt +++ b/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 )