Skip to content

Commit 94aa6fc

Browse files
gabrielluonggabrielluong
committed
Bug 1973366 - Part 5: Refactoring toolbar paste and go menu option to use FenixBrowserUseCases.loadUrlOrSearch r=android-reviewers,petru
- Refactors the existing usage of `HomeActivity.openToBrowserAndLoad` in `DefaultToolbarController.handlePasteAndGo` to use `FenixBrowserUseCases.loadUrlOrSearch`. - Paste and go menu item should open in the existing tab when Homepage as a New Tab is enabled. Differential Revision: https://phabricator.services.mozilla.com/D257962
1 parent a7c1e12 commit 94aa6fc

File tree

4 files changed

+105
-31
lines changed

4 files changed

+105
-31
lines changed

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import mozilla.components.concept.engine.EngineView
1717
import org.mozilla.fenix.R
1818
import org.mozilla.fenix.components.toolbar.ToolbarPosition
1919
import org.mozilla.fenix.ext.settings
20+
import org.mozilla.fenix.utils.Settings
2021
import java.lang.ref.WeakReference
2122

2223
/**
@@ -98,9 +99,17 @@ class BrowserAnimator(
9899

99100
companion object {
100101
fun getToolbarNavOptions(context: Context): NavOptions {
102+
return getToolbarNavOptions(context.settings().toolbarPosition)
103+
}
104+
105+
/**
106+
* Returns the [NavOptions] to be used when navigating based on the provided
107+
* [ToolbarPosition].
108+
*/
109+
fun getToolbarNavOptions(toolbarPosition: ToolbarPosition): NavOptions {
101110
val navOptions = NavOptions.Builder()
102111

103-
when (context.settings().toolbarPosition) {
112+
when (toolbarPosition) {
104113
ToolbarPosition.TOP -> {
105114
navOptions.setEnterAnim(R.anim.fade_in)
106115
navOptions.setExitAnim(R.anim.fade_out)

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,12 @@ class HomeFragment : Fragment() {
543543
navController = findNavController(),
544544
),
545545
toolbarController = DefaultToolbarController(
546-
activity = activity,
547-
store = components.core.store,
546+
appStore = components.appStore,
547+
browserStore = components.core.store,
548+
nimbusComponents = components.nimbus,
548549
navController = findNavController(),
550+
settings = components.settings,
551+
fenixBrowserUseCases = components.useCases.fenixBrowserUseCases,
549552
),
550553
homeSearchController = DefaultHomeSearchController(
551554
appStore = components.appStore,

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/ToolbarController.kt‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@ import androidx.navigation.NavController
88
import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine
99
import mozilla.components.browser.state.store.BrowserStore
1010
import mozilla.components.support.ktx.kotlin.isUrl
11-
import org.mozilla.fenix.BrowserDirection
1211
import org.mozilla.fenix.GleanMetrics.Events
13-
import org.mozilla.fenix.HomeActivity
1412
import org.mozilla.fenix.NavGraphDirections
13+
import org.mozilla.fenix.R
1514
import org.mozilla.fenix.browser.BrowserAnimator
15+
import org.mozilla.fenix.components.AppStore
16+
import org.mozilla.fenix.components.NimbusComponents
1617
import org.mozilla.fenix.components.metrics.MetricsUtils
17-
import org.mozilla.fenix.ext.components
18+
import org.mozilla.fenix.components.usecases.FenixBrowserUseCases
1819
import org.mozilla.fenix.ext.nav
20+
import org.mozilla.fenix.utils.Settings
1921

2022
/**
2123
* An interface that handles the view manipulation of the home screen toolbar.
@@ -41,29 +43,33 @@ interface ToolbarController {
4143
* The default implementation of [ToolbarController].
4244
*/
4345
class DefaultToolbarController(
44-
private val activity: HomeActivity,
45-
private val store: BrowserStore,
46+
private val appStore: AppStore,
47+
private val browserStore: BrowserStore,
48+
private val nimbusComponents: NimbusComponents,
4649
private val navController: NavController,
50+
private val settings: Settings,
51+
private val fenixBrowserUseCases: FenixBrowserUseCases,
4752
) : ToolbarController {
4853
override fun handlePasteAndGo(clipboardText: String) {
49-
val searchEngine = store.state.search.selectedOrDefaultSearchEngine
54+
val searchEngine = browserStore.state.search.selectedOrDefaultSearchEngine
5055

51-
activity.openToBrowserAndLoad(
56+
navController.navigate(R.id.browserFragment)
57+
fenixBrowserUseCases.loadUrlOrSearch(
5258
searchTermOrURL = clipboardText,
53-
newTab = true,
54-
from = BrowserDirection.FromHome,
55-
engine = searchEngine,
59+
newTab = !settings.enableHomepageAsNewTab,
60+
private = appStore.state.mode.isPrivate,
61+
searchEngine = searchEngine,
5662
)
5763

5864
if (clipboardText.isUrl() || searchEngine == null) {
5965
Events.enteredUrl.record(Events.EnteredUrlExtra(autocomplete = false))
6066
} else {
6167
val searchAccessPoint = MetricsUtils.Source.ACTION
6268
MetricsUtils.recordSearchMetrics(
63-
searchEngine,
64-
searchEngine == store.state.search.selectedOrDefaultSearchEngine,
65-
searchAccessPoint,
66-
activity.components.nimbus.events,
69+
engine = searchEngine,
70+
isDefault = searchEngine == browserStore.state.search.selectedOrDefaultSearchEngine,
71+
searchAccessPoint = searchAccessPoint,
72+
nimbusEventStore = nimbusComponents.events,
6773
)
6874
}
6975
}
@@ -84,7 +90,7 @@ class DefaultToolbarController(
8490
navController.nav(
8591
navController.currentDestination?.id,
8692
directions,
87-
BrowserAnimator.getToolbarNavOptions(activity),
93+
BrowserAnimator.getToolbarNavOptions(toolbarPosition = settings.toolbarPosition),
8894
)
8995

9096
Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME"))

‎mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/DefaultToolbarControllerTest.kt‎

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ package org.mozilla.fenix.home.toolbar
77
import androidx.navigation.NavController
88
import androidx.navigation.NavDirections
99
import androidx.navigation.NavOptions
10+
import io.mockk.Runs
1011
import io.mockk.every
12+
import io.mockk.just
1113
import io.mockk.mockk
1214
import io.mockk.verify
1315
import mozilla.components.browser.state.search.SearchEngine
@@ -22,11 +24,13 @@ import org.junit.Before
2224
import org.junit.Rule
2325
import org.junit.Test
2426
import org.junit.runner.RunWith
25-
import org.mozilla.fenix.BrowserDirection
27+
import org.mozilla.experiments.nimbus.NimbusEventStore
2628
import org.mozilla.fenix.GleanMetrics.Events
27-
import org.mozilla.fenix.HomeActivity
2829
import org.mozilla.fenix.R
29-
import org.mozilla.fenix.ext.settings
30+
import org.mozilla.fenix.components.AppStore
31+
import org.mozilla.fenix.components.NimbusComponents
32+
import org.mozilla.fenix.components.appstate.AppState
33+
import org.mozilla.fenix.components.usecases.FenixBrowserUseCases
3034
import org.mozilla.fenix.helpers.FenixGleanTestRule
3135
import org.mozilla.fenix.utils.Settings
3236
import org.robolectric.RobolectricTestRunner
@@ -37,9 +41,17 @@ class DefaultToolbarControllerTest {
3741
@get:Rule
3842
val gleanTestRule = FenixGleanTestRule(testContext)
3943

40-
private val activity: HomeActivity = mockk(relaxed = true)
44+
private val appStore: AppStore = mockk(relaxed = true)
4145
private val navController: NavController = mockk(relaxed = true)
4246
private val settings: Settings = mockk(relaxed = true)
47+
private val fenixBrowserUseCases: FenixBrowserUseCases = mockk(relaxed = true)
48+
49+
val nimbusEventsStore: NimbusEventStore = mockk {
50+
every { recordEvent(any()) } just Runs
51+
}
52+
private val nimbusComponents: NimbusComponents = mockk {
53+
every { events } returns nimbusEventsStore
54+
}
4355

4456
private val searchEngine = SearchEngine(
4557
id = "test",
@@ -61,10 +73,11 @@ class DefaultToolbarControllerTest {
6173
),
6274
)
6375

76+
every { appStore.state } returns AppState()
77+
6478
every { navController.currentDestination } returns mockk {
6579
every { id } returns R.id.homeFragment
6680
}
67-
every { activity.settings() } returns settings
6881
}
6982

7083
@Test
@@ -76,11 +89,12 @@ class DefaultToolbarControllerTest {
7689
createController().handlePasteAndGo(clipboardText)
7790

7891
verify {
79-
activity.openToBrowserAndLoad(
92+
navController.navigate(R.id.browserFragment)
93+
fenixBrowserUseCases.loadUrlOrSearch(
8094
searchTermOrURL = clipboardText,
8195
newTab = true,
82-
from = BrowserDirection.FromHome,
83-
engine = searchEngine,
96+
private = false,
97+
searchEngine = searchEngine,
8498
)
8599
}
86100

@@ -90,11 +104,50 @@ class DefaultToolbarControllerTest {
90104
createController().handlePasteAndGo(clipboardText)
91105

92106
verify {
93-
activity.openToBrowserAndLoad(
107+
navController.navigate(R.id.browserFragment)
108+
fenixBrowserUseCases.loadUrlOrSearch(
94109
searchTermOrURL = clipboardText,
95110
newTab = true,
96-
from = BrowserDirection.FromHome,
97-
engine = searchEngine,
111+
private = false,
112+
searchEngine = searchEngine,
113+
)
114+
}
115+
116+
assertNotNull(Events.enteredUrl.testGetValue())
117+
}
118+
119+
@Test
120+
fun `GIVEN homepage as a new tab is enabled WHEN Paste & Go toolbar menu is clicked THEN open the browser with the clipboard text as the search term`() {
121+
every { settings.enableHomepageAsNewTab } returns true
122+
123+
assertNull(Events.enteredUrl.testGetValue())
124+
assertNull(Events.performedSearch.testGetValue())
125+
126+
var clipboardText = "text"
127+
createController().handlePasteAndGo(clipboardText)
128+
129+
verify {
130+
navController.navigate(R.id.browserFragment)
131+
fenixBrowserUseCases.loadUrlOrSearch(
132+
searchTermOrURL = clipboardText,
133+
newTab = false,
134+
private = false,
135+
searchEngine = searchEngine,
136+
)
137+
}
138+
139+
assertNotNull(Events.performedSearch.testGetValue())
140+
141+
clipboardText = "https://mozilla.org"
142+
createController().handlePasteAndGo(clipboardText)
143+
144+
verify {
145+
navController.navigate(R.id.browserFragment)
146+
fenixBrowserUseCases.loadUrlOrSearch(
147+
searchTermOrURL = clipboardText,
148+
newTab = false,
149+
private = false,
150+
searchEngine = searchEngine,
98151
)
99152
}
100153

@@ -134,8 +187,11 @@ class DefaultToolbarControllerTest {
134187
}
135188

136189
private fun createController() = DefaultToolbarController(
137-
activity = activity,
138-
store = store,
190+
appStore = appStore,
191+
browserStore = store,
192+
nimbusComponents = nimbusComponents,
139193
navController = navController,
194+
settings = settings,
195+
fenixBrowserUseCases = fenixBrowserUseCases,
140196
)
141197
}

0 commit comments

Comments
 (0)