For #24210: Remove wrapper from "search bar tapped" event.
This commit is contained in:
parent
4c738cbf10
commit
51df82acb1
|
@ -47,6 +47,7 @@ events:
|
|||
description: |
|
||||
The view the user was on when they initiated the search (For example:
|
||||
`Home` or `Browser`)
|
||||
type: string
|
||||
bugs:
|
||||
- https://github.com/mozilla-mobile/fenix/issues/959
|
||||
- https://github.com/mozilla-mobile/fenix/issues/19923
|
||||
|
|
|
@ -284,13 +284,6 @@ sealed class Event {
|
|||
get() = hashMapOf(Logins.saveLoginsSettingChangedKeys.setting to setting.name)
|
||||
}
|
||||
|
||||
data class SearchBarTapped(val source: Source) : Event() {
|
||||
enum class Source { HOME, BROWSER }
|
||||
|
||||
override val extras: Map<Events.searchBarTappedKeys, String>?
|
||||
get() = mapOf(Events.searchBarTappedKeys.source to source.name)
|
||||
}
|
||||
|
||||
data class EnteredUrl(val autoCompleted: Boolean) : Event() {
|
||||
override val extras: Map<Events.enteredUrlKeys, String>?
|
||||
get() = mapOf(Events.enteredUrlKeys.autocomplete to autoCompleted.toString())
|
||||
|
|
|
@ -89,10 +89,6 @@ private class EventWrapper<T : Enum<T>>(
|
|||
// FIXME(#19967): Migrate to non-deprecated API.
|
||||
private val Event.wrapper: EventWrapper<*>?
|
||||
get() = when (this) {
|
||||
is Event.SearchBarTapped -> EventWrapper(
|
||||
{ Events.searchBarTapped.record(it) },
|
||||
{ Events.searchBarTappedKeys.valueOf(it) }
|
||||
)
|
||||
is Event.EnteredUrl -> EventWrapper(
|
||||
{ Events.enteredUrl.record(it) },
|
||||
{ Events.enteredUrlKeys.valueOf(it) }
|
||||
|
|
|
@ -15,6 +15,7 @@ import mozilla.components.concept.engine.EngineView
|
|||
import mozilla.components.feature.tabs.TabsUseCases
|
||||
import mozilla.components.support.ktx.kotlin.isUrl
|
||||
import mozilla.components.ui.tabcounter.TabCounterMenu
|
||||
import org.mozilla.fenix.GleanMetrics.Events
|
||||
import org.mozilla.fenix.HomeActivity
|
||||
import org.mozilla.fenix.R
|
||||
import org.mozilla.fenix.browser.BrowserAnimator
|
||||
|
@ -92,7 +93,7 @@ class DefaultBrowserToolbarController(
|
|||
}
|
||||
|
||||
override fun handleToolbarClick() {
|
||||
metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER))
|
||||
Events.searchBarTapped.record(Events.SearchBarTappedExtra("BROWSER"))
|
||||
// If we're displaying awesomebar search results, Home screen will not be visible (it's
|
||||
// covered up with the search results). So, skip the navigation event in that case.
|
||||
// If we don't, there's a visual flickr as we navigate to Home and then display search
|
||||
|
|
|
@ -76,6 +76,7 @@ import mozilla.components.ui.tabcounter.TabCounterMenu
|
|||
import org.mozilla.fenix.BrowserDirection
|
||||
import org.mozilla.fenix.Config
|
||||
import org.mozilla.fenix.FeatureFlags
|
||||
import org.mozilla.fenix.GleanMetrics.Events
|
||||
import org.mozilla.fenix.HomeActivity
|
||||
import org.mozilla.fenix.R
|
||||
import org.mozilla.fenix.browser.BrowserAnimator.Companion.getToolbarNavOptions
|
||||
|
@ -491,7 +492,6 @@ class HomeFragment : Fragment() {
|
|||
view.resources.getDimensionPixelSize(R.dimen.search_bar_search_engine_icon_padding)
|
||||
binding.toolbarWrapper.setOnClickListener {
|
||||
navigateToSearch()
|
||||
requireComponents.analytics.metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.HOME))
|
||||
}
|
||||
|
||||
binding.toolbarWrapper.setOnLongClickListener {
|
||||
|
@ -865,13 +865,16 @@ class HomeFragment : Fragment() {
|
|||
navigateToSearch()
|
||||
}
|
||||
|
||||
private fun navigateToSearch() {
|
||||
@VisibleForTesting
|
||||
internal fun navigateToSearch() {
|
||||
val directions =
|
||||
HomeFragmentDirections.actionGlobalSearchDialog(
|
||||
sessionId = null
|
||||
)
|
||||
|
||||
nav(R.id.homeFragment, directions, getToolbarNavOptions(requireContext()))
|
||||
|
||||
Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME"))
|
||||
}
|
||||
|
||||
@SuppressWarnings("ComplexMethod", "LongMethod")
|
||||
|
|
|
@ -25,17 +25,21 @@ import mozilla.components.feature.search.SearchUseCases
|
|||
import mozilla.components.feature.session.SessionUseCases
|
||||
import mozilla.components.feature.tabs.TabsUseCases
|
||||
import mozilla.components.feature.top.sites.TopSitesUseCases
|
||||
import mozilla.components.service.glean.testing.GleanTestRule
|
||||
import mozilla.components.support.test.ext.joinBlocking
|
||||
import mozilla.components.support.test.libstate.ext.waitUntilIdle
|
||||
import mozilla.components.support.test.middleware.CaptureActionsMiddleware
|
||||
import mozilla.components.support.test.robolectric.testContext
|
||||
import mozilla.components.ui.tabcounter.TabCounterMenu
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.mozilla.fenix.GleanMetrics.Events
|
||||
import org.mozilla.fenix.HomeActivity
|
||||
import org.mozilla.fenix.R
|
||||
import org.mozilla.fenix.browser.BrowserAnimator
|
||||
|
@ -91,6 +95,9 @@ class DefaultBrowserToolbarControllerTest {
|
|||
private lateinit var store: BrowserStore
|
||||
private val captureMiddleware = CaptureActionsMiddleware<BrowserState, BrowserAction>()
|
||||
|
||||
@get:Rule
|
||||
val gleanTestRule = GleanTestRule(testContext)
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
MockKAnnotations.init(this)
|
||||
|
@ -221,6 +228,8 @@ class DefaultBrowserToolbarControllerTest {
|
|||
@Test
|
||||
fun handleToolbarClick() {
|
||||
val controller = createController()
|
||||
assertFalse(Events.searchBarTapped.testHasValue())
|
||||
|
||||
controller.handleToolbarClick()
|
||||
|
||||
val homeDirections = BrowserFragmentDirections.actionGlobalHome()
|
||||
|
@ -228,9 +237,11 @@ class DefaultBrowserToolbarControllerTest {
|
|||
sessionId = "1"
|
||||
)
|
||||
|
||||
verify {
|
||||
metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER))
|
||||
}
|
||||
assertTrue(Events.searchBarTapped.testHasValue())
|
||||
val snapshot = Events.searchBarTapped.testGetValue()
|
||||
assertEquals(1, snapshot.size)
|
||||
assertEquals("BROWSER", snapshot.single().extra?.getValue("source"))
|
||||
|
||||
verify {
|
||||
// shows the home screen "behind" the search dialog
|
||||
navController.navigate(homeDirections)
|
||||
|
@ -243,6 +254,8 @@ class DefaultBrowserToolbarControllerTest {
|
|||
val searchResultsTab = createTab("https://google.com?q=mozilla+website", searchTerms = "mozilla website")
|
||||
store.dispatch(TabListAction.AddTabAction(searchResultsTab, select = true)).joinBlocking()
|
||||
|
||||
assertFalse(Events.searchBarTapped.testHasValue())
|
||||
|
||||
val controller = createController()
|
||||
controller.handleToolbarClick()
|
||||
|
||||
|
@ -251,9 +264,11 @@ class DefaultBrowserToolbarControllerTest {
|
|||
sessionId = searchResultsTab.id
|
||||
)
|
||||
|
||||
verify {
|
||||
metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER))
|
||||
}
|
||||
assertTrue(Events.searchBarTapped.testHasValue())
|
||||
val snapshot = Events.searchBarTapped.testGetValue()
|
||||
assertEquals(1, snapshot.size)
|
||||
assertEquals("BROWSER", snapshot.single().extra?.getValue("source"))
|
||||
|
||||
// Does not show the home screen "behind" the search dialog if the current session has search terms.
|
||||
verify(exactly = 0) {
|
||||
navController.navigate(homeDirections)
|
||||
|
|
Loading…
Reference in New Issue
Block a user