diff --git a/app/metrics.yaml b/app/metrics.yaml index 2d418d5f6..ceacceaec 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -82,7 +82,7 @@ events: A string containing the name of the item the user tapped. These items include: Settings, Library, Help, Desktop Site toggle on/off, Find in Page, New Tab, Private Tab, Share, Report Site Issue, Back/Forward button, Reload Button, Quit, - Reader Mode On, Reader Mode Off, Open In App + Reader Mode On, Reader Mode Off, Open In App, Add to Firefox Home bugs: - https://github.com/mozilla-mobile/fenix/issues/1024 data_reviews: 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 df79e6529..3af8972b1 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -78,7 +78,6 @@ import org.mozilla.fenix.downloads.DownloadNotificationBottomSheetDialog import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.enterToImmersiveMode -import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav @@ -153,19 +152,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session val store = context.components.core.store return getSessionById()?.also { session -> - - // We need to show the snackbar while the browsing data is deleting(if "Delete - // browsing data on quit" is activated). After the deletion is over, the snackbar - // is dismissed. - val snackbar: FenixSnackbar? = requireActivity().getRootView()?.let { v -> - FenixSnackbar.makeWithToolbarPadding(v) - .setText(v.context.getString(R.string.deleting_browsing_data_in_progress)) - } - val browserToolbarController = DefaultBrowserToolbarController( store = browserFragmentStore, activity = requireActivity(), - snackbar = snackbar, navController = findNavController(), readerModeController = DefaultReaderModeController( readerViewFeature, @@ -192,7 +181,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session }, bookmarkTapped = { lifecycleScope.launch { bookmarkTapped(it) } }, scope = lifecycleScope, - tabCollectionStorage = requireComponents.core.tabCollectionStorage + tabCollectionStorage = requireComponents.core.tabCollectionStorage, + topSiteStorage = requireComponents.core.topSiteStorage ) browserInteractor = BrowserInteractor( diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt index f0d329afb..1c0cfe002 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt @@ -337,8 +337,8 @@ sealed class Event { enum class Item { SETTINGS, LIBRARY, HELP, DESKTOP_VIEW_ON, DESKTOP_VIEW_OFF, FIND_IN_PAGE, NEW_TAB, NEW_PRIVATE_TAB, SHARE, REPORT_SITE_ISSUE, BACK, FORWARD, RELOAD, STOP, OPEN_IN_FENIX, - SAVE_TO_COLLECTION, ADD_TO_HOMESCREEN, QUIT, READER_MODE_ON, READER_MODE_OFF, OPEN_IN_APP, - BOOKMARK, READER_MODE_APPEARANCE + SAVE_TO_COLLECTION, ADD_TO_FIREFOX_HOME, ADD_TO_HOMESCREEN, QUIT, READER_MODE_ON, + READER_MODE_OFF, OPEN_IN_APP, BOOKMARK, READER_MODE_APPEARANCE } override val extras: Map? diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index 20627e1e9..152bcbb08 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -11,12 +11,14 @@ import android.graphics.drawable.ColorDrawable import android.view.View import android.view.ViewGroup import androidx.annotation.VisibleForTesting -import androidx.lifecycle.LifecycleCoroutineScope import androidx.navigation.NavController import androidx.navigation.NavDirections import androidx.navigation.NavOptions import androidx.navigation.fragment.FragmentNavigator import androidx.swiperefreshlayout.widget.SwipeRefreshLayout +import com.google.android.material.snackbar.Snackbar +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.MainScope import kotlinx.coroutines.launch @@ -35,8 +37,10 @@ import org.mozilla.fenix.browser.readermode.ReaderModeController import org.mozilla.fenix.collections.SaveCollectionStep import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.components.TopSiteStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.nav import org.mozilla.fenix.lib.Do import org.mozilla.fenix.settings.deletebrowsingdata.deleteAndQuit @@ -56,7 +60,6 @@ interface BrowserToolbarController { class DefaultBrowserToolbarController( private val store: BrowserFragmentStore, private val activity: Activity, - private val snackbar: FenixSnackbar?, private val navController: NavController, private val readerModeController: ReaderModeController, private val browsingModeManager: BrowsingModeManager, @@ -70,13 +73,20 @@ class DefaultBrowserToolbarController( private val getSupportUrl: () -> String, private val openInFenixIntent: Intent, private val bookmarkTapped: (Session) -> Unit, - private val scope: LifecycleCoroutineScope, - private val tabCollectionStorage: TabCollectionStorage + private val scope: CoroutineScope, + private val tabCollectionStorage: TabCollectionStorage, + private val topSiteStorage: TopSiteStorage ) : BrowserToolbarController { private val currentSession get() = customTabSession ?: activity.components.core.sessionManager.selectedSession + // We hold onto a reference of the inner scope so that we can override this with the + // TestCoroutineScope to ensure sequential execution. If we didn't have this, our tests + // would fail intermittently due to the async nature of coroutine scheduling. + @VisibleForTesting + internal var ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO) + override fun handleToolbarPaste(text: String) { adjustBackgroundAndNavigate.invoke( BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( @@ -131,6 +141,19 @@ class DefaultBrowserToolbarController( item.isChecked, currentSession ) + ToolbarMenu.Item.AddToFirefoxHome -> { + ioScope.launch { + currentSession?.let { + topSiteStorage.addTopSite(it.title, it.url) + } + + activity.getRootView()?.let { + FenixSnackbar.makeWithToolbarPadding(it, Snackbar.LENGTH_SHORT) + .setText(it.context.getString(R.string.snackbar_added_to_firefox_home)) + .show() + } + } + } ToolbarMenu.Item.AddToHomeScreen -> { MainScope().launch { with(activity.components.useCases.webAppUseCases) { @@ -211,7 +234,17 @@ class DefaultBrowserToolbarController( // Close this activity since it is no longer displaying any session activity.finish() } - ToolbarMenu.Item.Quit -> deleteAndQuit(activity, scope, snackbar) + ToolbarMenu.Item.Quit -> { + // We need to show the snackbar while the browsing data is deleting (if "Delete + // browsing data on quit" is activated). After the deletion is over, the snackbar + // is dismissed. + val snackbar: FenixSnackbar? = activity.getRootView()?.let { v -> + FenixSnackbar.makeWithToolbarPadding(v) + .setText(v.context.getString(R.string.deleting_browsing_data_in_progress)) + } + + deleteAndQuit(activity, scope, snackbar) + } is ToolbarMenu.Item.ReaderMode -> { val enabled = currentSession?.readerMode ?: activity.components.core.sessionManager.selectedSession?.readerMode @@ -289,6 +322,7 @@ class DefaultBrowserToolbarController( ToolbarMenu.Item.OpenInFenix -> Event.BrowserMenuItemTapped.Item.OPEN_IN_FENIX ToolbarMenu.Item.Share -> Event.BrowserMenuItemTapped.Item.SHARE ToolbarMenu.Item.SaveToCollection -> Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION + ToolbarMenu.Item.AddToFirefoxHome -> Event.BrowserMenuItemTapped.Item.ADD_TO_FIREFOX_HOME ToolbarMenu.Item.AddToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN ToolbarMenu.Item.Quit -> Event.BrowserMenuItemTapped.Item.QUIT is ToolbarMenu.Item.ReaderMode -> 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 60c8da98b..ea28de186 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 @@ -155,6 +155,7 @@ class DefaultToolbarMenu( settings, library, desktopMode, + addToFirefoxHome, addToHomescreen.apply { visible = ::shouldShowAddToHomescreen }, findInPage, privateTab, @@ -214,6 +215,14 @@ class DefaultToolbarMenu( onItemTapped.invoke(ToolbarMenu.Item.RequestDesktop(checked)) } + private val addToFirefoxHome = BrowserMenuImageText( + label = context.getString(R.string.browser_menu_add_to_firefox_home), + imageResource = R.drawable.ic_home, + iconTintColorResource = primaryTextColor() + ) { + onItemTapped.invoke(ToolbarMenu.Item.AddToFirefoxHome) + } + private val addToHomescreen = BrowserMenuHighlightableItem( label = context.getString(R.string.browser_menu_add_to_homescreen), startImageResource = R.drawable.ic_add_to_homescreen, 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 acf2eaeeb..ced4a10ce 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 @@ -24,6 +24,7 @@ interface ToolbarMenu { object ReportIssue : Item() object OpenInFenix : Item() object SaveToCollection : Item() + object AddToFirefoxHome : Item() object AddToHomeScreen : Item() object Quit : Item() data class ReaderMode(val isChecked: Boolean) : Item() diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index 3e108e306..642185bc0 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt @@ -23,6 +23,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ObsoleteCoroutinesApi import kotlinx.coroutines.newSingleThreadContext import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runBlockingTest import kotlinx.coroutines.test.setMain import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager @@ -44,6 +45,7 @@ import org.mozilla.fenix.collections.SaveCollectionStep import org.mozilla.fenix.components.Analytics import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.components.TopSiteStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components @@ -76,6 +78,7 @@ class DefaultBrowserToolbarControllerTest { private val adjustBackgroundAndNavigate: (NavDirections) -> Unit = mockk(relaxed = true) private val snackbar = mockk(relaxed = true) private val tabCollectionStorage = mockk(relaxed = true) + private val topSiteStorage = mockk(relaxed = true) private lateinit var controller: DefaultBrowserToolbarController @@ -85,7 +88,6 @@ class DefaultBrowserToolbarControllerTest { controller = DefaultBrowserToolbarController( activity = activity, - snackbar = snackbar, navController = navController, browsingModeManager = browsingModeManager, findInPageLauncher = findInPageLauncher, @@ -98,6 +100,7 @@ class DefaultBrowserToolbarControllerTest { browserLayout = browserLayout, swipeRefresh = swipeRefreshLayout, tabCollectionStorage = tabCollectionStorage, + topSiteStorage = topSiteStorage, bookmarkTapped = mockk(), readerModeController = mockk(), sessionManager = mockk(), @@ -291,6 +294,37 @@ class DefaultBrowserToolbarControllerTest { } } + @Test + fun handleToolbarAddToFirefoxHomePress() = runBlockingTest { + val item = ToolbarMenu.Item.AddToFirefoxHome + + controller = DefaultBrowserToolbarController( + activity = activity, + navController = navController, + browsingModeManager = browsingModeManager, + findInPageLauncher = findInPageLauncher, + engineView = engineView, + adjustBackgroundAndNavigate = adjustBackgroundAndNavigate, + customTabSession = null, + getSupportUrl = getSupportUrl, + openInFenixIntent = openInFenixIntent, + scope = this, + browserLayout = browserLayout, + swipeRefresh = swipeRefreshLayout, + tabCollectionStorage = tabCollectionStorage, + topSiteStorage = topSiteStorage, + bookmarkTapped = mockk(), + readerModeController = mockk(), + sessionManager = mockk(), + store = mockk() + ) + controller.ioScope = this + + controller.handleToolbarItemInteraction(item) + + verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.ADD_TO_FIREFOX_HOME)) } + } + @Test fun handleToolbarAddToHomeScreenPress() { val item = ToolbarMenu.Item.AddToHomeScreen @@ -453,7 +487,6 @@ class DefaultBrowserToolbarControllerTest { fun handleToolbarOpenInFenixPress() { controller = DefaultBrowserToolbarController( activity = activity, - snackbar = snackbar, navController = navController, browsingModeManager = browsingModeManager, findInPageLauncher = findInPageLauncher, @@ -466,6 +499,7 @@ class DefaultBrowserToolbarControllerTest { browserLayout = browserLayout, swipeRefresh = swipeRefreshLayout, tabCollectionStorage = tabCollectionStorage, + topSiteStorage = topSiteStorage, bookmarkTapped = mockk(), readerModeController = mockk(), sessionManager = mockk(), @@ -489,11 +523,33 @@ class DefaultBrowserToolbarControllerTest { } @Test - fun handleToolbarQuitPress() { + fun handleToolbarQuitPress() = runBlockingTest { val item = ToolbarMenu.Item.Quit + val testScope = this + + controller = DefaultBrowserToolbarController( + activity = activity, + navController = navController, + browsingModeManager = browsingModeManager, + findInPageLauncher = findInPageLauncher, + engineView = engineView, + adjustBackgroundAndNavigate = adjustBackgroundAndNavigate, + customTabSession = null, + getSupportUrl = getSupportUrl, + openInFenixIntent = openInFenixIntent, + scope = testScope, + browserLayout = browserLayout, + swipeRefresh = swipeRefreshLayout, + tabCollectionStorage = tabCollectionStorage, + topSiteStorage = topSiteStorage, + bookmarkTapped = mockk(), + readerModeController = mockk(), + sessionManager = mockk(), + store = mockk() + ) controller.handleToolbarItemInteraction(item) - verify { deleteAndQuit(activity, scope, snackbar) } + verify { deleteAndQuit(activity, testScope, null) } } }