diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchController.kt b/app/src/main/java/org/mozilla/fenix/search/SearchController.kt index 5a01ba3ce..062c07322 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchController.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchController.kt @@ -7,9 +7,6 @@ package org.mozilla.fenix.search import android.content.Intent import androidx.navigation.NavController -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.delay -import kotlinx.coroutines.launch import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session import mozilla.components.support.ktx.kotlin.isUrl @@ -51,7 +48,6 @@ class DefaultSearchController( private val activity: HomeActivity, private val store: SearchFragmentStore, private val navController: NavController, - private val viewLifecycleScope: CoroutineScope, private val clearToolbarFocus: () -> Unit ) : SearchController { @@ -101,13 +97,7 @@ class DefaultSearchController( } override fun handleEditingCancelled() { - viewLifecycleScope.launch { - clearToolbarFocus() - // Delay a short amount so the keyboard begins animating away. This makes exit animation - // much smoother instead of having two separate parts (keyboard hides THEN animation) - delay(KEYBOARD_ANIMATION_DELAY) - navController.popBackStack() - } + clearToolbarFocus() } override fun handleTextChanged(text: String) { @@ -199,8 +189,4 @@ class DefaultSearchController( handleExistingSessionSelected(session) } } - - companion object { - internal const val KEYBOARD_ANIMATION_DELAY = 5L - } } diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt index 6495c6a16..280f411e0 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt @@ -24,7 +24,6 @@ import androidx.core.content.ContextCompat import androidx.core.view.isVisible import androidx.core.widget.NestedScrollView import androidx.fragment.app.Fragment -import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import kotlinx.android.synthetic.main.fragment_search.* @@ -120,7 +119,6 @@ class SearchFragment : Fragment(), UserInteractionHandler { activity = activity, store = searchStore, navController = findNavController(), - viewLifecycleScope = viewLifecycleOwner.lifecycleScope, clearToolbarFocus = ::clearToolbarFocus ) @@ -182,6 +180,7 @@ class SearchFragment : Fragment(), UserInteractionHandler { } private fun clearToolbarFocus() { + toolbarView.view.hideKeyboard() toolbarView.view.clearFocus() } @@ -347,14 +346,14 @@ class SearchFragment : Fragment(), UserInteractionHandler { } override fun onBackPressed(): Boolean { - // Note: Actual navigation happens in `handleEditingCancelled` in SearchController return when { qrFeature.onBackPressed() -> { toolbarView.view.edit.focus() view?.search_scan_button?.isChecked = false toolbarView.view.requestFocus() + true } - else -> true + else -> false } } @@ -426,7 +425,6 @@ class SearchFragment : Fragment(), UserInteractionHandler { } companion object { - private const val SHARED_TRANSITION_MS = 250L private const val REQUEST_CODE_CAMERA_PERMISSIONS = 1 } } diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt index 624734297..bf997e413 100644 --- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt @@ -59,7 +59,6 @@ class ToolbarView( ) { private var isInitialized = false - private var hasBeenCanceled = false init { view.apply { @@ -96,19 +95,18 @@ class ToolbarView( ) edit.setUrlBackground( - AppCompatResources.getDrawable(context, R.drawable.search_url_background)) + AppCompatResources.getDrawable(context, R.drawable.search_url_background) + ) private = isPrivate setOnEditListener(object : mozilla.components.concept.toolbar.Toolbar.OnEditListener { override fun onCancelEditing(): Boolean { - // For some reason, this can be triggered twice on one back press. This only leads to - // navigateUp, so let's make sure we only call it once - if (!hasBeenCanceled) interactor.onEditingCanceled() - hasBeenCanceled = true + interactor.onEditingCanceled() // We need to return false to not show display mode return false } + override fun onTextChanged(text: String) { url = text this@ToolbarView.interactor.onTextChanged(text) @@ -144,13 +142,15 @@ class ToolbarView( isInitialized = true } - val iconSize = context.resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) + val iconSize = + context.resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) val scaledIcon = Bitmap.createScaledBitmap( searchState.searchEngineSource.searchEngine.icon, iconSize, iconSize, - true) + true + ) val icon = BitmapDrawable(context.resources, scaledIcon) diff --git a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt index d5f9d159a..b2bc035e2 100644 --- a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.search -import androidx.lifecycle.LifecycleCoroutineScope import androidx.navigation.NavController import androidx.navigation.NavDirections import io.mockk.every @@ -32,7 +31,6 @@ import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.searchEngineManager import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -import org.mozilla.fenix.search.DefaultSearchController.Companion.KEYBOARD_ANIMATION_DELAY import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.whatsnew.clear @@ -49,7 +47,6 @@ class DefaultSearchControllerTest { private val searchEngine: SearchEngine = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) private val sessionManager: SessionManager = mockk(relaxed = true) - private val lifecycleScope: LifecycleCoroutineScope = mockk(relaxed = true) private val clearToolbarFocus: (() -> Unit) = mockk(relaxed = true) private lateinit var controller: DefaultSearchController @@ -67,7 +64,6 @@ class DefaultSearchControllerTest { activity = activity, store = store, navController = navController, - viewLifecycleScope = lifecycleScope, clearToolbarFocus = clearToolbarFocus ) @@ -123,17 +119,13 @@ class DefaultSearchControllerTest { activity = activity, store = store, navController = navController, - viewLifecycleScope = this, clearToolbarFocus = clearToolbarFocus ) controller.handleEditingCancelled() - advanceTimeBy(KEYBOARD_ANIMATION_DELAY) - verify { clearToolbarFocus() - navController.popBackStack() } } diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt index e92a38f9b..82b69a425 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchInteractorTest.kt @@ -4,249 +4,84 @@ package org.mozilla.fenix.search -import android.content.Context -import androidx.lifecycle.LifecycleCoroutineScope -import androidx.navigation.NavController -import androidx.navigation.NavDirections -import io.mockk.Runs -import io.mockk.every -import io.mockk.just import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.SearchEngineManager import mozilla.components.browser.session.Session +import org.junit.Before import org.junit.Test -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.FenixApplication -import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore.PREF_FILE_SEARCH_ENGINES -import org.mozilla.fenix.ext.metrics -import org.mozilla.fenix.ext.navigateSafe -import org.mozilla.fenix.ext.searchEngineManager @ExperimentalCoroutinesApi class SearchInteractorTest { - private val lifecycleScope: LifecycleCoroutineScope = mockk(relaxed = true) - private val clearToolbarFocus = { } + lateinit var searchController: DefaultSearchController + lateinit var interactor: SearchInteractor + + @Before + fun setup() { + searchController = mockk(relaxed = true) + interactor = SearchInteractor( + searchController + ) + } @Test fun onUrlCommitted() { - val context: HomeActivity = mockk(relaxed = true) - val store: SearchFragmentStore = mockk() - val state: SearchFragmentState = mockk() - val searchEngineManager: SearchEngineManager = mockk(relaxed = true) - val searchEngine = SearchEngineSource.Default(mockk(relaxed = true)) - val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint = mockk(relaxed = true) - - every { context.metrics } returns mockk(relaxed = true) - every { context.searchEngineManager } returns searchEngineManager - every { context.openToBrowserAndLoad(any(), any(), any(), any(), any(), any()) } just Runs - - every { store.state } returns state - every { state.session } returns null - every { state.searchEngineSource } returns searchEngine - every { state.searchAccessPoint } returns searchAccessPoint - - every { - context.getSharedPreferences( - PREF_FILE_SEARCH_ENGINES, - Context.MODE_PRIVATE - ) - } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - context, - store, - mockk(), - lifecycleScope, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) - interactor.onUrlCommitted("test") verify { - context.openToBrowserAndLoad( - searchTermOrURL = "test", - newTab = true, - from = BrowserDirection.FromSearch, - engine = searchEngine.searchEngine - ) + searchController.handleUrlCommitted("test") } } @Test fun onEditingCanceled() = runBlockingTest { - val navController: NavController = mockk(relaxed = true) - val store: SearchFragmentStore = mockk(relaxed = true) - - every { store.state } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - mockk(), - store, - navController, - this, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) - interactor.onEditingCanceled() - advanceTimeBy(DefaultSearchController.KEYBOARD_ANIMATION_DELAY) verify { - clearToolbarFocus() - navController.popBackStack() + searchController.handleEditingCancelled() } } @Test fun onTextChanged() { - val store: SearchFragmentStore = mockk(relaxed = true) - val context: HomeActivity = mockk(relaxed = true) - - every { store.state } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - context, - store, - mockk(), - lifecycleScope, - clearToolbarFocus - ) val interactor = SearchInteractor(searchController) interactor.onTextChanged("test") - verify { store.dispatch(SearchFragmentAction.UpdateQuery("test")) } - verify { store.dispatch(SearchFragmentAction.AllowSearchSuggestionsInPrivateModePrompt(false)) } + verify { searchController.handleTextChanged("test") } } @Test fun onUrlTapped() { - val context: HomeActivity = mockk() - val store: SearchFragmentStore = mockk() - val state: SearchFragmentState = mockk() - val searchEngine = SearchEngineSource.Default(mockk(relaxed = true)) - - every { context.metrics } returns mockk(relaxed = true) - every { context.openToBrowserAndLoad(any(), any(), any()) } just Runs - - every { store.state } returns state - every { state.session } returns null - every { state.searchEngineSource } returns searchEngine - - every { - context.getSharedPreferences( - PREF_FILE_SEARCH_ENGINES, - Context.MODE_PRIVATE - ) - } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - context, - store, - mockk(), - lifecycleScope, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) - interactor.onUrlTapped("test") verify { - context.openToBrowserAndLoad( - "test", - true, - BrowserDirection.FromSearch - ) + searchController.handleUrlTapped("test") } } @Test fun onSearchTermsTapped() { - val context: HomeActivity = mockk(relaxed = true) - val store: SearchFragmentStore = mockk() - val state: SearchFragmentState = mockk() - val searchEngineManager: SearchEngineManager = mockk(relaxed = true) - val searchEngine = SearchEngineSource.Default(mockk(relaxed = true)) - val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint = mockk(relaxed = true) - - every { context.metrics } returns mockk(relaxed = true) - every { context.searchEngineManager } returns searchEngineManager - every { context.openToBrowserAndLoad(any(), any(), any(), any(), any(), any()) } just Runs - - every { store.state } returns state - every { state.session } returns null - every { state.searchEngineSource } returns searchEngine - every { state.searchAccessPoint } returns searchAccessPoint - - every { - context.getSharedPreferences( - PREF_FILE_SEARCH_ENGINES, - Context.MODE_PRIVATE - ) - } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - context, - store, - mockk(), - lifecycleScope, - clearToolbarFocus - ) - - val interactor = SearchInteractor(searchController) - interactor.onSearchTermsTapped("test") verify { - context.openToBrowserAndLoad( - searchTermOrURL = "test", - newTab = true, - from = BrowserDirection.FromSearch, - engine = searchEngine.searchEngine, - forceSearch = true - ) + searchController.handleSearchTermsTapped("test") } } @Test fun onSearchShortcutEngineSelected() { - val context: HomeActivity = mockk(relaxed = true) - - every { context.metrics } returns mockk(relaxed = true) - - val store: SearchFragmentStore = mockk(relaxed = true) - val state: SearchFragmentState = mockk(relaxed = true) - - every { store.state } returns state - - val searchController: SearchController = DefaultSearchController( - context, - store, - mockk(), - lifecycleScope, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) val searchEngine: SearchEngine = mockk(relaxed = true) interactor.onSearchShortcutEngineSelected(searchEngine) - verify { store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine)) } + verify { searchController.handleSearchShortcutEngineSelected(searchEngine) } } @Test fun onSearchShortcutsButtonClicked() { - val searchController: SearchController = mockk(relaxed = true) - val interactor = SearchInteractor(searchController) - interactor.onSearchShortcutsButtonClicked() verify { searchController.handleSearchShortcutsButtonClicked() } @@ -254,58 +89,21 @@ class SearchInteractorTest { @Test fun onClickSearchEngineSettings() { - val navController: NavController = mockk() - val store: SearchFragmentStore = mockk() - - every { store.state } returns mockk(relaxed = true) - every { navController.currentDestination?.id } returns R.id.searchFragment - - val searchController: SearchController = DefaultSearchController( - mockk(), - store, - navController, - lifecycleScope, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) - - every { navController.navigate(any() as NavDirections) } just Runs - interactor.onClickSearchEngineSettings() verify { - navController.navigateSafe( - R.id.searchFragment, - SearchFragmentDirections.actionGlobalSearchEngineFragment() - ) + searchController.handleClickSearchEngineSettings() } } @Test fun onExistingSessionSelected() { - val navController: NavController = mockk(relaxed = true) - val context: HomeActivity = mockk(relaxed = true) - val applicationContext: FenixApplication = mockk(relaxed = true) - every { context.applicationContext } returns applicationContext - val store: SearchFragmentStore = mockk() - every { context.openToBrowser(any(), any()) } just Runs - - every { store.state } returns mockk(relaxed = true) - - val searchController: SearchController = DefaultSearchController( - context, - store, - navController, - lifecycleScope, - clearToolbarFocus - ) - val interactor = SearchInteractor(searchController) val session = Session("http://mozilla.org", false) interactor.onExistingSessionSelected(session) verify { - context.openToBrowser(BrowserDirection.FromSearch) + searchController.handleExistingSessionSelected(session) } } }