From 1ed9ebf622d4b517a06b02a737e3ad420e1f4a0a Mon Sep 17 00:00:00 2001 From: ekager Date: Tue, 10 Nov 2020 17:26:41 -0800 Subject: [PATCH] For #16132 - Rename normal mode menu item, remove telemetry --- app/metrics.yaml | 3 +-- .../mozilla/fenix/ui/TabbedBrowsingTest.kt | 4 +-- .../fenix/ui/robots/ThreeDotMenuMainRobot.kt | 5 ++++ .../fenix/tabtray/MultiselectSelectionMenu.kt | 4 --- .../fenix/tabtray/TabTrayController.kt | 27 ++++++++++++++----- .../fenix/tabtray/TabTrayDialogFragment.kt | 4 ++- .../mozilla/fenix/tabtray/TabTrayItemMenu.kt | 11 ++++---- .../org/mozilla/fenix/tabtray/TabTrayView.kt | 4 +-- .../tabtray/DefaultTabTrayControllerTest.kt | 7 ++++- docs/metrics.md | 2 +- 10 files changed, 45 insertions(+), 26 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 8002047611..d18976de8e 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2200,8 +2200,7 @@ tabs_tray: save_to_collection: type: event description: | - A user tapped the save to collection button in the - three dot menu within the tabs tray + A user tapped the save to collection button in the tabs tray bugs: - https://github.com/mozilla-mobile/fenix/issues/11273 data_reviews: diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/TabbedBrowsingTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/TabbedBrowsingTest.kt index d8d460537b..24d78cc619 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/TabbedBrowsingTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/TabbedBrowsingTest.kt @@ -85,7 +85,7 @@ class TabbedBrowsingTest { }.openTabsListThreeDotMenu { verifyCloseAllTabsButton() verifyShareTabButton() - verifySaveCollection() + verifySelectTabs() } } @@ -127,7 +127,7 @@ class TabbedBrowsingTest { }.openTabsListThreeDotMenu { verifyCloseAllTabsButton() verifyShareTabButton() - verifySaveCollection() + verifySelectTabs() }.closeAllTabs { verifyNoTabsOpened() } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt index ac22377818..03e6ec5bf1 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt @@ -69,6 +69,7 @@ class ThreeDotMenuMainRobot { fun verifyShareTabButton() = assertShareTabButton() fun verifySaveCollection() = assertSaveCollectionButton() + fun verifySelectTabs() = assertSelectTabsButton() fun clickBrowserViewSaveCollectionButton() { browserViewSaveCollectionButton().click() @@ -430,6 +431,10 @@ private fun saveCollectionButton() = onView(allOf(withText("Save to collection") private fun assertSaveCollectionButton() = saveCollectionButton() .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) +private fun selectTabsButton() = onView(allOf(withText("Select tabs"))).inRoot(RootMatchers.isPlatformPopup()) +private fun assertSelectTabsButton() = selectTabsButton() + .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) + private fun addNewCollectionButton() = onView(allOf(withText("Add new collection"))) private fun assertaddNewCollectionButton() = addNewCollectionButton() .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/MultiselectSelectionMenu.kt b/app/src/main/java/org/mozilla/fenix/tabtray/MultiselectSelectionMenu.kt index c2c8b8c836..7f4acbb834 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/MultiselectSelectionMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/MultiselectSelectionMenu.kt @@ -8,8 +8,6 @@ import android.content.Context import mozilla.components.browser.menu.BrowserMenuBuilder import mozilla.components.browser.menu.item.SimpleBrowserMenuItem import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.components class MultiselectSelectionMenu( private val context: Context, @@ -28,7 +26,6 @@ class MultiselectSelectionMenu( context.getString(R.string.tab_tray_multiselect_menu_item_bookmark), textColorResource = R.color.primary_text_normal_theme ) { - context.components.analytics.metrics.track(Event.TabsTraySaveToCollectionPressed) onItemTapped.invoke(Item.BookmarkTabs) }, @@ -36,7 +33,6 @@ class MultiselectSelectionMenu( context.getString(R.string.tab_tray_multiselect_menu_item_close), textColorResource = R.color.primary_text_normal_theme ) { - context.components.analytics.metrics.track(Event.TabsTrayShareAllTabsPressed) onItemTapped.invoke(Item.DeleteTabs) } ) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt index d5f2c48169..bb3a4f56b4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt @@ -6,7 +6,6 @@ package org.mozilla.fenix.tabtray import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import mozilla.appservices.places.BookmarkRoot @@ -14,6 +13,7 @@ import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.selector.normalTabs +import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.base.profiler.Profiler import mozilla.components.concept.engine.prompt.ShareData @@ -25,6 +25,8 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.home.HomeFragment import mozilla.components.browser.storage.sync.Tab as SyncTab @@ -58,19 +60,27 @@ interface TabTrayController { /** * Default behavior of [TabTrayController]. Other implementations are possible. * + * @param activity [Activity] the current activity. * @param profiler [Profiler] used for profiling. * @param sessionManager [HomeActivity] used for retrieving a list of sessions. + * @param browserStore [BrowserStore] holds the global [BrowserState]. * @param browsingModeManager [HomeActivity] used for registering browsing mode. - * @param navController [NavController] used for navigation. + * @param tabCollectionStorage [TabCollectionStorage] storage for saving collections. + * @param ioScope [CoroutineScope] with an IO dispatcher used for structured concurrency. + * @param metrics reference to the configured [MetricController] to record telemetry events. + * @param tabsUseCases [TabsUseCases] use cases related to the tabs feature. + * @param navController - [NavController] used for navigation. * @param dismissTabTray callback allowing to request this entire Fragment to be dismissed. - * @param tabTrayDialogFragmentStore [TabTrayDialogFragmentStore] holding the State for all Views displayed - * in this Controller's Fragment. * @param dismissTabTrayAndNavigateHome callback allowing showing an undo snackbar after tab deletion. - * @param selectTabUseCase [TabsUseCases.SelectTabUseCase] callback allowing for selecting a tab. * @param registerCollectionStorageObserver callback allowing for registering the [TabCollectionStorage.Observer] * when needed. + * @param tabTrayDialogFragmentStore [TabTrayDialogFragmentStore] holding the State for all Views displayed + * in this Controller's Fragment. + * @param selectTabUseCase [TabsUseCases.SelectTabUseCase] callback allowing for selecting a tab. * @param showChooseCollectionDialog callback allowing saving a list of sessions to an existing collection. * @param showAddNewCollectionDialog callback allowing for saving a list of sessions to a new collection. + * @param showUndoSnackbarForTabs callback allowing for showing an undo snackbar for removed tabs. + * @param showBookmarksSnackbar callback allowing for showing a snackbar with action to view bookmarks. */ @Suppress("TooManyFunctions") class DefaultTabTrayController( @@ -81,7 +91,8 @@ class DefaultTabTrayController( private val browsingModeManager: BrowsingModeManager, private val tabCollectionStorage: TabCollectionStorage, private val bookmarksStorage: BookmarksStorage, - private val scope: CoroutineScope, + private val ioScope: CoroutineScope, + private val metrics: MetricController, private val tabsUseCases: TabsUseCases, private val navController: NavController, private val dismissTabTray: () -> Unit, @@ -115,6 +126,8 @@ class DefaultTabTrayController( } override fun handleSaveToCollectionClicked(selectedTabs: Set) { + metrics.track(Event.TabsTraySaveToCollectionPressed) + val sessionList = selectedTabs.map { sessionManager.findSessionById(it.id) ?: return } @@ -155,7 +168,7 @@ class DefaultTabTrayController( override fun handleBookmarkSelectedTabs(selectedTabs: Set) { selectedTabs.forEach { - scope.launch(IO) { + ioScope.launch { val shouldAddBookmark = bookmarksStorage.getBookmarksWithUrl(it.url) .firstOrNull { it.url == it.url } == null if (shouldAddBookmark) { diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt index 0c0e7f667e..c0dcd4d385 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt @@ -30,6 +30,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch +import kotlinx.coroutines.plus import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session import mozilla.components.browser.state.selector.findTab @@ -199,7 +200,8 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler sessionManager = activity.components.core.sessionManager, browserStore = activity.components.core.store, tabsUseCases = activity.components.useCases.tabsUseCases, - scope = lifecycleScope, + ioScope = lifecycleScope + Dispatchers.IO, + metrics = activity.components.analytics.metrics, browsingModeManager = activity.browsingModeManager, tabCollectionStorage = activity.components.core.tabCollectionStorage, bookmarksStorage = activity.components.core.bookmarksStorage, diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayItemMenu.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayItemMenu.kt index 8a855c0912..f7469337b9 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayItemMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayItemMenu.kt @@ -13,7 +13,7 @@ import org.mozilla.fenix.ext.components class TabTrayItemMenu( private val context: Context, - private val shouldShowSaveToCollection: () -> Boolean, + private val shouldShowSelectTabs: () -> Boolean, private val hasOpenTabs: () -> Boolean, private val onItemTapped: (Item) -> Unit = {} ) { @@ -21,7 +21,7 @@ class TabTrayItemMenu( sealed class Item { object ShareAllTabs : Item() object OpenTabSettings : Item() - object SaveToCollection : Item() + object SelectTabs : Item() object CloseAllTabs : Item() object OpenRecentlyClosed : Item() } @@ -31,12 +31,11 @@ class TabTrayItemMenu( private val menuItems by lazy { listOf( SimpleBrowserMenuItem( - context.getString(R.string.tab_tray_menu_item_save), + context.getString(R.string.tabs_tray_select_tabs), textColorResource = R.color.primary_text_normal_theme ) { - context.components.analytics.metrics.track(Event.TabsTraySaveToCollectionPressed) - onItemTapped.invoke(Item.SaveToCollection) - }.apply { visible = shouldShowSaveToCollection }, + onItemTapped.invoke(Item.SelectTabs) + }.apply { visible = shouldShowSelectTabs }, SimpleBrowserMenuItem( context.getString(R.string.tab_tray_menu_item_share), diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index ed6494e4c9..398d20504f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -226,7 +226,7 @@ class TabTrayView( tabTrayItemMenu = TabTrayItemMenu( context = view.context, - shouldShowSaveToCollection = { checkOpenTabs.invoke() && view.tab_layout.selectedTabPosition == 0 }, + shouldShowSelectTabs = { checkOpenTabs.invoke() && view.tab_layout.selectedTabPosition == 0 }, hasOpenTabs = checkOpenTabs ) { when (it) { @@ -234,7 +234,7 @@ class TabTrayView( isPrivateModeSelected ) is TabTrayItemMenu.Item.OpenTabSettings -> interactor.onTabSettingsClicked() - is TabTrayItemMenu.Item.SaveToCollection -> interactor.onEnterMultiselect() + is TabTrayItemMenu.Item.SelectTabs -> interactor.onEnterMultiselect() is TabTrayItemMenu.Item.CloseAllTabs -> interactor.onCloseAllTabsClicked( isPrivateModeSelected ) diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt index 7648cf96be..284bb0de9d 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt @@ -38,6 +38,8 @@ import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.sessionsOfType @OptIn(ExperimentalCoroutinesApi::class) @@ -70,6 +72,7 @@ class DefaultTabTrayControllerTest { private val tabsUseCases: TabsUseCases = mockk(relaxed = true) private val showUndoSnackbarForTabs: (() -> Unit) = mockk(relaxed = true) private val showBookmarksSavedSnackbar: (() -> Unit) = mockk(relaxed = true) + private val metrics: MetricController = mockk(relaxed = true) private lateinit var controller: DefaultTabTrayController @@ -109,7 +112,8 @@ class DefaultTabTrayControllerTest { browsingModeManager = browsingModeManager, tabCollectionStorage = tabCollectionStorage, bookmarksStorage = bookmarksStorage, - scope = TestCoroutineScope(), + ioScope = TestCoroutineScope(), + metrics = metrics, navController = navController, tabsUseCases = tabsUseCases, dismissTabTray = dismissTabTray, @@ -268,6 +272,7 @@ class DefaultTabTrayControllerTest { controller.handleSaveToCollectionClicked(setOf(tab)) verify { + metrics.track(Event.TabsTraySaveToCollectionPressed) registerCollectionStorageObserver() showChooseCollectionDialog(listOf(session)) } diff --git a/docs/metrics.md b/docs/metrics.md index 47b249a037..a9a0a9a8d1 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -204,7 +204,7 @@ The following metrics are added to the ping: | tabs_tray.opened |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user opened the tabs tray |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | | tabs_tray.opened_existing_tab |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user opened an existing tab |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | | tabs_tray.private_mode_tapped |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user switched to private mode |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | -| tabs_tray.save_to_collection |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user tapped the save to collection button in the three dot menu within the tabs tray |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | +| tabs_tray.save_to_collection |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user tapped the save to collection button in the tabs tray |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | | tabs_tray.share_all_tabs |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user tapped the share all tabs button in the three dot menu within the tabs tray |[1](https://github.com/mozilla-mobile/fenix/pull/12036), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | | tip.closed |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |The tip was closed |[1](https://github.com/mozilla-mobile/fenix/pull/9836), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)|
  • identifier: The identifier of the tip closed
|2021-08-01 |2 | | tip.displayed |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |The tip was displayed |[1](https://github.com/mozilla-mobile/fenix/pull/9836), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)|
  • identifier: The identifier of the tip displayed
|2021-08-01 |2 |