diff --git a/app/metrics.yaml b/app/metrics.yaml index 5b57fe956e..003f505232 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2625,7 +2625,11 @@ tabs_tray: opened_existing_tab: type: event description: | - A user opened an existing tab + A user opened an existing tab from a particular app feature. + extra_keys: + source: + description: | + From which app feature was an existing tab opened. bugs: - https://github.com/mozilla-mobile/fenix/issues/11273 data_reviews: @@ -2633,15 +2637,20 @@ tabs_tray: - https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068 - https://github.com/mozilla-mobile/fenix/pull/19924#issuecomment-861423789 - https://github.com/mozilla-mobile/fenix/pull/20517#pullrequestreview-718069041 + - https://github.com/mozilla-mobile/fenix/pull/20508#issuecomment-902160532 data_sensitivity: - interaction notification_emails: - android-probes@mozilla.com - expires: "2022-02-01" + expires: "2022-08-01" closed_existing_tab: type: event description: | - A user closed an existing tab + A user closed an existing tab from a particular app feature. + extra_keys: + source: + description: | + From which app feature was an existing tab closed. bugs: - https://github.com/mozilla-mobile/fenix/issues/11273 data_reviews: @@ -2649,11 +2658,12 @@ tabs_tray: - https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068 - https://github.com/mozilla-mobile/fenix/pull/19924#issuecomment-861423789 - https://github.com/mozilla-mobile/fenix/pull/20517#pullrequestreview-718069041 + - https://github.com/mozilla-mobile/fenix/pull/20508#issuecomment-902160532 data_sensitivity: - interaction notification_emails: - android-probes@mozilla.com - expires: "2022-02-01" + expires: "2022-08-01" private_mode_tapped: type: event description: | @@ -2811,6 +2821,34 @@ tabs_tray: notification_emails: - android-probes@mozilla.com expires: "2022-08-01" + inactive_tabs_expanded: + type: event + description: | + A user tapped the "Inactive tabs" header to expand + the group of inactive tabs. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/20507 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/20508#issuecomment-901336677 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: "2022-08-01" + inactive_tabs_collapsed: + type: event + description: | + A user tapped the "Inactive tabs" header to collapse + the group of inactive tabs. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/20507 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/20508#issuecomment-901336677 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: "2022-08-01" collections: renamed: 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 0e627b7e46..bb50f927b5 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 @@ -21,6 +21,7 @@ import org.mozilla.fenix.GleanMetrics.Logins import org.mozilla.fenix.GleanMetrics.Onboarding import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp import org.mozilla.fenix.GleanMetrics.SearchShortcuts +import org.mozilla.fenix.GleanMetrics.TabsTray import org.mozilla.fenix.GleanMetrics.Tip import org.mozilla.fenix.GleanMetrics.ToolbarSettings import org.mozilla.fenix.GleanMetrics.TopSites @@ -175,8 +176,12 @@ sealed class Event { // Tab tray object TabsTrayOpened : Event() object TabsTrayClosed : Event() - object OpenedExistingTab : Event() - object ClosedExistingTab : Event() + data class OpenedExistingTab(val source: String) : Event() { + override val extras = mapOf(TabsTray.openedExistingTabKeys.source to source) + } + data class ClosedExistingTab(val source: String) : Event() { + override val extras = mapOf(TabsTray.closedExistingTabKeys.source to source) + } object TabsTrayPrivateModeTapped : Event() object TabsTrayNormalModeTapped : Event() object TabsTraySyncedModeTapped : Event() @@ -187,6 +192,8 @@ sealed class Event { object TabsTrayShareAllTabsPressed : Event() object TabsTrayCloseAllTabsPressed : Event() object TabsTrayRecentlyClosedPressed : Event() + object TabsTrayInactiveTabsExpanded : Event() + object TabsTrayInactiveTabsCollapsed : Event() object ProgressiveWebAppOpenFromHomescreenTap : Event() object ProgressiveWebAppInstallAsShortcut : Event() diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 1e3fc18b57..7472e4f0ec 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -650,11 +650,13 @@ private val Event.wrapper: EventWrapper<*>? is Event.TabsTrayClosed -> EventWrapper( { TabsTray.closed.record(it) } ) - is Event.OpenedExistingTab -> EventWrapper( - { TabsTray.openedExistingTab.record(it) } + is Event.OpenedExistingTab -> EventWrapper( + { TabsTray.openedExistingTab.record(it) }, + { TabsTray.openedExistingTabKeys.valueOf(it) } ) - is Event.ClosedExistingTab -> EventWrapper( - { TabsTray.closedExistingTab.record(it) } + is Event.ClosedExistingTab -> EventWrapper( + { TabsTray.closedExistingTab.record(it) }, + { TabsTray.closedExistingTabKeys.valueOf(it) } ) is Event.TabsTrayPrivateModeTapped -> EventWrapper( { TabsTray.privateModeTapped.record(it) } @@ -686,6 +688,12 @@ private val Event.wrapper: EventWrapper<*>? is Event.TabsTrayRecentlyClosedPressed -> EventWrapper( { TabsTray.inactiveTabsRecentlyClosed.record(it) } ) + is Event.TabsTrayInactiveTabsExpanded -> EventWrapper( + { TabsTray.inactiveTabsExpanded.record(it) } + ) + is Event.TabsTrayInactiveTabsCollapsed -> EventWrapper( + { TabsTray.inactiveTabsCollapsed.record(it) } + ) is Event.AutoPlaySettingVisited -> EventWrapper( { Autoplay.visitedSetting.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt index cbeeb757b2..da04b28098 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt @@ -35,11 +35,11 @@ class TrayPagerAdapter( private val normalAdapter by lazy { ConcatAdapter( - BrowserTabsAdapter(context, browserInteractor, store), - InactiveTabsAdapter(context, browserInteractor) + BrowserTabsAdapter(context, browserInteractor, store, TABS_TRAY_FEATURE_NAME), + InactiveTabsAdapter(context, browserInteractor, INACTIVE_TABS_FEATURE_NAME) ) } - private val privateAdapter by lazy { BrowserTabsAdapter(context, browserInteractor, store) } + private val privateAdapter by lazy { BrowserTabsAdapter(context, browserInteractor, store, TABS_TRAY_FEATURE_NAME) } private val syncedTabsAdapter by lazy { SyncedTabsAdapter(TabClickDelegate(navInteractor)) } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AbstractPageViewHolder { @@ -98,6 +98,10 @@ class TrayPagerAdapter( companion object { const val TRAY_TABS_COUNT = 3 + // Telemetry keys for identifying from which app features the a was opened / closed. + const val TABS_TRAY_FEATURE_NAME = "Tabs tray" + const val INACTIVE_TABS_FEATURE_NAME = "Inactive tabs" + val POSITION_NORMAL_TABS = Page.NormalTabs.ordinal val POSITION_PRIVATE_TABS = Page.PrivateTabs.ordinal val POSITION_SYNCED_TABS = Page.SyncedTabs.ordinal diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt index 02b81a09b9..34c5c1dbd6 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt @@ -35,19 +35,28 @@ import org.mozilla.fenix.ext.removeTouchDelegate import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.selection.SelectionHolder -import org.mozilla.fenix.selection.SelectionInteractor import org.mozilla.fenix.tabstray.TabsTrayState import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.tabstray.ext.isSelect /** * A RecyclerView ViewHolder implementation for "tab" items. + * + * @param itemView [View] that displays a "tab". + * @param imageLoader [ImageLoader] used to load tab thumbnails. + * @param trayStore [TabsTrayStore] containing the complete state of tabs tray and methods to update that. + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. + * @param store [BrowserStore] containing the complete state of the browser and methods to update that. + * @param metrics [MetricController] used for handling telemetry events. */ +@Suppress("LongParameterList") abstract class AbstractBrowserTabViewHolder( itemView: View, private val imageLoader: ImageLoader, private val trayStore: TabsTrayStore, private val selectionHolder: SelectionHolder?, + @VisibleForTesting + internal val featureName: String, private val store: BrowserStore = itemView.context.components.core.store, private val metrics: MetricController = itemView.context.components.analytics.metrics ) : TabViewHolder(itemView) { @@ -91,7 +100,7 @@ abstract class AbstractBrowserTabViewHolder( if (selectionHolder != null) { setSelectionInteractor(tab, selectionHolder, browserTrayInteractor) } else { - itemView.setOnClickListener { browserTrayInteractor.open(tab) } + itemView.setOnClickListener { browserTrayInteractor.open(tab, featureName) } } if (tab.thumbnail != null) { @@ -202,12 +211,12 @@ abstract class AbstractBrowserTabViewHolder( private fun setSelectionInteractor( item: Tab, holder: SelectionHolder, - interactor: SelectionInteractor + interactor: BrowserTrayInteractor ) { itemView.setOnClickListener { val selected = holder.selectedItems when { - selected.isEmpty() && trayStore.state.mode.isSelect().not() -> interactor.open(item) + selected.isEmpty() && trayStore.state.mode.isSelect().not() -> interactor.open(item, featureName) item in selected -> interactor.deselect(item) else -> interactor.select(item) } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabGridViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabGridViewHolder.kt index 46637def81..5ca700e27c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabGridViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabGridViewHolder.kt @@ -21,14 +21,22 @@ import org.mozilla.fenix.tabstray.TabsTrayStore /** * A RecyclerView ViewHolder implementation for "tab" items with grid layout. + * + * @param imageLoader [ImageLoader] used to load tab thumbnails. + * @param browserTrayInteractor [BrowserTrayInteractor] handling tabs interactions in a tab tray. + * @param store [TabsTrayStore] containing the complete state of tabs tray and methods to update that. + * @param selectionHolder [SelectionHolder]<[Tab]> for helping with selecting any number of displayed [Tab]s. + * @param itemView [View] that displays a "tab". + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. */ class BrowserTabGridViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, store: TabsTrayStore, selectionHolder: SelectionHolder? = null, - itemView: View -) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder) { + itemView: View, + featureName: String +) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { private val closeButton: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabListViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabListViewHolder.kt index da3f13ba8b..9e217c29c2 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabListViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabListViewHolder.kt @@ -15,14 +15,22 @@ import kotlin.math.max /** * A RecyclerView ViewHolder implementation for "tab" items with list layout. - */ + * + * @param imageLoader [ImageLoader] used to load tab thumbnails. + * @param browserTrayInteractor [BrowserTrayInteractor] handling tabs interactions in a tab tray. + * @param store [TabsTrayStore] containing the complete state of tabs tray and methods to update that. + * @param selectionHolder [SelectionHolder]<[Tab]> for helping with selecting any number of displayed [Tab]s. + * @param itemView [View] that displays a "tab". + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. +*/ class BrowserTabListViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, store: TabsTrayStore, selectionHolder: SelectionHolder? = null, - itemView: View -) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder) { + itemView: View, + featureName: String +) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { override val thumbnailSize: Int get() = max( itemView.resources.getDimensionPixelSize(R.dimen.tab_tray_list_item_thumbnail_height), diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt index 66199ea5c2..9e89a2123f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt @@ -17,6 +17,7 @@ import mozilla.components.concept.tabstray.TabsTray import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry import org.mozilla.fenix.R +import org.mozilla.fenix.components.Components import org.mozilla.fenix.databinding.TabTrayGridItemBinding import org.mozilla.fenix.databinding.TabTrayItemBinding import org.mozilla.fenix.ext.components @@ -25,11 +26,18 @@ import org.mozilla.fenix.tabstray.TabsTrayStore /** * A [RecyclerView.Adapter] for browser tabs. + * + * @param context [Context] used for various platform interactions or accessing [Components] + * @param interactor [BrowserTrayInteractor] handling tabs interactions in a tab tray. + * @param store [TabsTrayStore] containing the complete state of tabs tray and methods to update that. + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. + * @param delegate [Observable]<[TabsTray.Observer]> for observing tabs tray changes. Defaults to [ObserverRegistry]. */ class BrowserTabsAdapter( private val context: Context, private val interactor: BrowserTrayInteractor, private val store: TabsTrayStore, + private val featureName: String, delegate: Observable = ObserverRegistry() ) : TabsAdapter(delegate) { @@ -62,9 +70,9 @@ class BrowserTabsAdapter( return when (viewType) { ViewType.GRID.layoutRes -> - BrowserTabGridViewHolder(imageLoader, interactor, store, selectionHolder, view) + BrowserTabGridViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) else -> - BrowserTabListViewHolder(imageLoader, interactor, store, selectionHolder, view) + BrowserTabListViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) } } @@ -76,12 +84,12 @@ class BrowserTabsAdapter( ViewType.GRID.layoutRes -> { val gridBinding = TabTrayGridItemBinding.bind(holder.itemView) selectedMaskView = gridBinding.checkboxInclude.selectedMask - gridBinding.mozacBrowserTabstrayClose.setOnClickListener { interactor.close(tab) } + gridBinding.mozacBrowserTabstrayClose.setOnClickListener { interactor.close(tab, featureName) } } ViewType.LIST.layoutRes -> { val listBinding = TabTrayItemBinding.bind(holder.itemView) selectedMaskView = listBinding.checkboxInclude.selectedMask - listBinding.mozacBrowserTabstrayClose.setOnClickListener { interactor.close(tab) } + listBinding.mozacBrowserTabstrayClose.setOnClickListener { interactor.close(tab, featureName) } } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt index d1c4ad46f9..02c4fc2117 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt @@ -21,10 +21,21 @@ import org.mozilla.fenix.tabstray.TabsTrayStore */ interface BrowserTrayInteractor : SelectionInteractor, UserInteractionHandler { + /** + * Open a tab. + * + * @param tab [Tab] to open in browser. + * @param source app feature from which the [tab] was opened. + */ + fun open(tab: Tab, source: String? = null) + /** * Close the tab. + * + * @param tab [Tab] to close. + * @param source app feature from which the [tab] was closed. */ - fun close(tab: Tab) + fun close(tab: Tab, source: String? = null) /** * TabTray's Floating Action Button clicked. @@ -65,14 +76,21 @@ class DefaultBrowserTrayInteractor( * See [SelectionInteractor.open] */ override fun open(item: Tab) { - selectTabWrapper.invoke(item.id) + open(item, null) + } + + /** + * See [BrowserTrayInteractor.open]. + */ + override fun open(tab: Tab, source: String?) { + selectTabWrapper.invoke(tab.id, source) } /** * See [BrowserTrayInteractor.close]. */ - override fun close(tab: Tab) { - removeTabWrapper.invoke(tab.id) + override fun close(tab: Tab, source: String?) { + removeTabWrapper.invoke(tab.id, source) } /** diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt index 5726b3f34b..68fee1b8e5 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt @@ -52,9 +52,17 @@ sealed class InactiveTabViewHolder(itemView: View) : RecyclerView.ViewHolder(ite } } + /** + * A RecyclerView ViewHolder implementation for an inactive tab view. + * + * @param itemView the inactive tab [View]. + * @param browserTrayInteractor [BrowserTrayInteractor] handling tabs interactions in a tab tray. + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. + */ class TabViewHolder( itemView: View, - private val browserTrayInteractor: BrowserTrayInteractor + private val browserTrayInteractor: BrowserTrayInteractor, + private val featureName: String ) : InactiveTabViewHolder(itemView) { private val binding = InactiveTabListItemBinding.bind(itemView) @@ -65,7 +73,7 @@ sealed class InactiveTabViewHolder(itemView: View) : RecyclerView.ViewHolder(ite val url = tab.url.toShortUrl(components.publicSuffixList).take(MAX_URI_LENGTH) itemView.setOnClickListener { - browserTrayInteractor.open(tab) + browserTrayInteractor.open(tab, featureName) } binding.siteListItem.apply { @@ -75,7 +83,7 @@ sealed class InactiveTabViewHolder(itemView: View) : RecyclerView.ViewHolder(ite R.drawable.mozac_ic_close, R.string.content_description_close_button ) { - browserTrayInteractor.close(tab) + browserTrayInteractor.close(tab, featureName) } } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt index 165f6e3332..cc7e1377f5 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt @@ -13,6 +13,7 @@ import mozilla.components.concept.tabstray.Tab as TabsTrayTab import mozilla.components.concept.tabstray.Tabs import mozilla.components.concept.tabstray.TabsTray import mozilla.components.support.base.observer.ObserverRegistry +import org.mozilla.fenix.components.Components import org.mozilla.fenix.tabstray.browser.InactiveTabViewHolder.FooterHolder import org.mozilla.fenix.tabstray.browser.InactiveTabViewHolder.HeaderHolder import org.mozilla.fenix.tabstray.browser.InactiveTabViewHolder.RecentlyClosedHolder @@ -32,10 +33,16 @@ private typealias Observable = ComponentObservable /** * The [ListAdapter] for displaying the list of inactive tabs. + * + * @param context [Context] used for various platform interactions or accessing [Components] + * @param browserTrayInteractor [BrowserTrayInteractor] handling tabs interactions in a tab tray. + * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. + * @param delegate [Observable]<[TabsTray.Observer]> for observing tabs tray changes. Defaults to [ObserverRegistry]. */ class InactiveTabsAdapter( private val context: Context, private val browserTrayInteractor: BrowserTrayInteractor, + private val featureName: String, delegate: Observable = ObserverRegistry() ) : Adapter(DiffCallback), TabsTray, Observable by delegate { @@ -47,7 +54,7 @@ class InactiveTabsAdapter( return when (viewType) { HeaderHolder.LAYOUT_ID -> HeaderHolder(view, inactiveTabsInteractor) - TabViewHolder.LAYOUT_ID -> TabViewHolder(view, browserTrayInteractor) + TabViewHolder.LAYOUT_ID -> TabViewHolder(view, browserTrayInteractor, featureName) FooterHolder.LAYOUT_ID -> FooterHolder(view) RecentlyClosedHolder.LAYOUT_ID -> RecentlyClosedHolder(view, browserTrayInteractor) else -> throw IllegalStateException("Unknown viewType: $viewType") diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt index 16f9e94b65..2658894eee 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt @@ -8,11 +8,14 @@ import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.tabstray.TabsTray import mozilla.components.feature.tabs.ext.toTabs +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.components.metrics.Event class InactiveTabsController( private val browserStore: BrowserStore, private val tabFilter: (TabSessionState) -> Boolean, - private val tray: TabsTray + private val tray: TabsTray, + private val metrics: MetricController ) { /** * Updates the inactive card to be expanded to display all the tabs, or collapsed with only @@ -21,6 +24,13 @@ class InactiveTabsController( fun updateCardExpansion(isExpanded: Boolean) { InactiveTabsState.isExpanded = isExpanded + metrics.track( + when (isExpanded) { + true -> Event.TabsTrayInactiveTabsExpanded + false -> Event.TabsTrayInactiveTabsCollapsed + } + ) + val tabs = browserStore.state.toTabs { tabFilter.invoke(it) } tray.updateTabs(tabs) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt index 9e12a46a42..3b4cd2e4eb 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt @@ -68,7 +68,7 @@ class NormalBrowserTrayList @JvmOverloads constructor( } val tabsAdapter = concatAdapter.inactiveTabsAdapter.apply { inactiveTabsInteractor = DefaultInactiveTabsInteractor( - InactiveTabsController(store, tabFilter, this) + InactiveTabsController(store, tabFilter, this, context.components.analytics.metrics) ) } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/UseCases.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/UseCases.kt index 377453bb86..dbb69d236f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/UseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/UseCases.kt @@ -13,19 +13,27 @@ class SelectTabUseCaseWrapper( private val selectTab: TabsUseCases.SelectTabUseCase, private val onSelect: (String) -> Unit ) : TabsUseCases.SelectTabUseCase { - override fun invoke(tabId: String) { - metrics.track(Event.OpenedExistingTab) + operator fun invoke(tabId: String, source: String? = null) { + metrics.track(Event.OpenedExistingTab(source ?: "unknown")) selectTab(tabId) onSelect(tabId) } + + override fun invoke(tabId: String) { + invoke(tabId, null) + } } class RemoveTabUseCaseWrapper( private val metrics: MetricController, - private val onRemove: (String) -> Unit + private val onRemove: (String) -> Unit, ) : TabsUseCases.RemoveTabUseCase { - override fun invoke(tabId: String) { - metrics.track(Event.ClosedExistingTab) + operator fun invoke(tabId: String, source: String? = null) { + metrics.track(Event.ClosedExistingTab(source ?: "unknown")) onRemove(tabId) } + + override fun invoke(tabId: String) { + invoke(tabId, null) + } } diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt index 3be0067944..12e6170fb9 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -207,12 +207,24 @@ class GleanMetricsServiceTest { assertTrue(TabsTray.closed.testHasValue()) assertFalse(TabsTray.openedExistingTab.testHasValue()) - gleanService.track(Event.OpenedExistingTab) + gleanService.track(Event.OpenedExistingTab("Test")) assertTrue(TabsTray.openedExistingTab.testHasValue()) + var events = TabsTray.openedExistingTab.testGetValue() + assertEquals(1, events.size) + assertEquals("tabs_tray", events[0].category) + assertEquals("opened_existing_tab", events[0].name) + assertEquals(1, events[0].extra!!.size) + assertEquals("Test", events[0].extra!!["source"]) assertFalse(TabsTray.closedExistingTab.testHasValue()) - gleanService.track(Event.ClosedExistingTab) + gleanService.track(Event.ClosedExistingTab("Test")) assertTrue(TabsTray.closedExistingTab.testHasValue()) + events = TabsTray.closedExistingTab.testGetValue() + assertEquals(1, events.size) + assertEquals("tabs_tray", events[0].category) + assertEquals("closed_existing_tab", events[0].name) + assertEquals(1, events[0].extra!!.size) + assertEquals("Test", events[0].extra!!["source"]) assertFalse(TabsTray.privateModeTapped.testHasValue()) gleanService.track(Event.TabsTrayPrivateModeTapped) diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolderTest.kt index 3adc606014..7dd6ec983b 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolderTest.kt @@ -44,7 +44,7 @@ class AbstractBrowserTabViewHolderTest { holder.itemView.performClick() - verify { interactor.open(any()) } + verify { interactor.open(any(), holder.featureName) } } @Test @@ -65,7 +65,7 @@ class AbstractBrowserTabViewHolderTest { holder.itemView.performClick() - verify { interactor.open(any()) } + verify { interactor.open(any(), holder.featureName) } assertTrue(selectionHolder.invoked) } @@ -77,8 +77,9 @@ class AbstractBrowserTabViewHolderTest { selectionHolder: SelectionHolder?, store: BrowserStore, metrics: MetricController, - override val browserTrayInteractor: BrowserTrayInteractor - ) : AbstractBrowserTabViewHolder(itemView, imageLoader, trayStore, selectionHolder, store, metrics) { + override val browserTrayInteractor: BrowserTrayInteractor, + featureName: String = "Test" + ) : AbstractBrowserTabViewHolder(itemView, imageLoader, trayStore, selectionHolder, featureName, store, metrics) { override val thumbnailSize: Int get() = 30 diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapterTest.kt index b790574a9b..3217584294 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapterTest.kt @@ -30,7 +30,7 @@ class BrowserTabsAdapterTest { @Test fun `WHEN bind with payloads is called THEN update the holder`() { - val adapter = BrowserTabsAdapter(context, interactor, store) + val adapter = BrowserTabsAdapter(context, interactor, store, "Test") val holder = mockk(relaxed = true) adapter.updateTabs( @@ -53,7 +53,7 @@ class BrowserTabsAdapterTest { @Test fun `WHEN the selection holder is set THEN update the selected tab`() { - val adapter = BrowserTabsAdapter(context, interactor, store) + val adapter = BrowserTabsAdapter(context, interactor, store, "Test") val binding = TabTrayItemBinding.inflate(LayoutInflater.from(testContext)) val holder = spyk( BrowserTabListViewHolder( @@ -61,7 +61,8 @@ class BrowserTabsAdapterTest { browserTrayInteractor = interactor, store = store, selectionHolder = null, - itemView = binding.root + itemView = binding.root, + featureName = "Test" ) ) val tab = createTab("tab1") diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt index 207f619fa4..7656e20a1e 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt @@ -15,6 +15,8 @@ import mozilla.components.concept.tabstray.TabsTray import org.junit.Assert.assertEquals import mozilla.components.browser.state.state.createTab as createTabState import org.junit.Test +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController class InactiveTabsControllerTest { @Test @@ -31,7 +33,7 @@ class InactiveTabsControllerTest { ) val tray: TabsTray = mockk(relaxed = true) val tabsSlot = slot() - val controller = InactiveTabsController(store, filter, tray) + val controller = InactiveTabsController(store, filter, tray, mockk(relaxed = true)) controller.updateCardExpansion(true) @@ -40,4 +42,30 @@ class InactiveTabsControllerTest { assertEquals(2, tabsSlot.captured.list.size) assertEquals("1", tabsSlot.captured.list.first().id) } + + @Test + fun `WHEN expanded THEN track telemetry event`() { + val metrics: MetricController = mockk(relaxed = true) + val store = BrowserStore(BrowserState()) + val controller = InactiveTabsController( + store, mockk(relaxed = true), mockk(relaxed = true), metrics + ) + + controller.updateCardExpansion(true) + + verify { metrics.track(Event.TabsTrayInactiveTabsExpanded) } + } + + @Test + fun `WHEN collapsed THEN track telemetry event`() { + val metrics: MetricController = mockk(relaxed = true) + val store = BrowserStore(BrowserState()) + val controller = InactiveTabsController( + store, mockk(relaxed = true), mockk(relaxed = true), metrics + ) + + controller.updateCardExpansion(false) + + verify { metrics.track(Event.TabsTrayInactiveTabsCollapsed) } + } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/RemoveTabUseCaseWrapperTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/RemoveTabUseCaseWrapperTest.kt index 022b05bd47..fe594f6be6 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/RemoveTabUseCaseWrapperTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/RemoveTabUseCaseWrapperTest.kt @@ -16,7 +16,7 @@ class RemoveTabUseCaseWrapperTest { val metricController = mockk(relaxed = true) @Test - fun `WHEN invoked THEN metrics, use case and callback are triggered`() { + fun `WHEN invoked with no source name THEN metrics with unknown source, use case and callback are triggered`() { var actualTabId: String? = null val onRemove: (String) -> Unit = { tabId -> actualTabId = tabId @@ -25,7 +25,21 @@ class RemoveTabUseCaseWrapperTest { wrapper("123") - verify { metricController.track(Event.ClosedExistingTab) } + verify { metricController.track(Event.ClosedExistingTab("unknown")) } + assertEquals("123", actualTabId) + } + + @Test + fun `WHEN invoked with a source name THEN metrics containing the source, use case and callback are triggered`() { + var actualTabId: String? = null + val onRemove: (String) -> Unit = { tabId -> + actualTabId = tabId + } + val wrapper = RemoveTabUseCaseWrapper(metricController, onRemove) + + wrapper("123", "Test") + + verify { metricController.track(Event.ClosedExistingTab("Test")) } assertEquals("123", actualTabId) } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/SelectTabUseCaseWrapperTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/SelectTabUseCaseWrapperTest.kt index fd0f245746..7e82c3ac94 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/SelectTabUseCaseWrapperTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/SelectTabUseCaseWrapperTest.kt @@ -17,13 +17,25 @@ class SelectTabUseCaseWrapperTest { val selectUseCase: TabsUseCases.SelectTabUseCase = mockk(relaxed = true) @Test - fun `WHEN invoked THEN metrics, use case and callback are triggered`() { + fun `WHEN invoked with no source name THEN metrics with unknown source, use case and callback are triggered`() { val onSelect: (String) -> Unit = mockk(relaxed = true) val wrapper = SelectTabUseCaseWrapper(metricController, selectUseCase, onSelect) wrapper("123") - verify { metricController.track(Event.OpenedExistingTab) } + verify { metricController.track(Event.OpenedExistingTab("unknown")) } + verify { selectUseCase("123") } + verify { onSelect("123") } + } + + @Test + fun `WHEN invoked with a source name THEN metrics, use case and callback are triggered`() { + val onSelect: (String) -> Unit = mockk(relaxed = true) + val wrapper = SelectTabUseCaseWrapper(metricController, selectUseCase, onSelect) + + wrapper("123", "Test") + + verify { metricController.track(Event.OpenedExistingTab("Test")) } verify { selectUseCase("123") } verify { onSelect("123") } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt index aef750d17a..9bbbacbf07 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt @@ -28,7 +28,7 @@ class AbstractBrowserPageViewHolderTest { val store: TabsTrayStore = TabsTrayStore() val interactor = mockk(relaxed = true) val browserTrayInteractor = mockk(relaxed = true) - val adapter = BrowserTabsAdapter(testContext, browserTrayInteractor, store) + val adapter = BrowserTabsAdapter(testContext, browserTrayInteractor, store, "Test") @Test fun `WHEN tabs inserted THEN show tray`() {