For #6758 - Part 4: Implement "Add to Firefox Home" browser menu item
- The "Add to Firefox Home" browser menu item adds a top site to the top site storage. - Refactors the FenixSnackbar from BaseBrowserFragment into BrowserToolbarController since there are multiple menu items that need to show a FenixSnackbar. - Adds metrics for the new browser menu item.
This commit is contained in:
parent
9ddf93eb7d
commit
e1863dd3c2
|
@ -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:
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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<Events.browserMenuActionKeys, String>?
|
||||
|
|
|
@ -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 ->
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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<FenixSnackbar>(relaxed = true)
|
||||
private val tabCollectionStorage = mockk<TabCollectionStorage>(relaxed = true)
|
||||
private val topSiteStorage = mockk<TopSiteStorage>(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) }
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue
Block a user