Do not show home screen behind search if we have search terms

Home screen isn't actually visible in case we're displaying awesomebar search results. The navigation is thus unnecessary and actually causes visual jankiness as we display home for a moment before covering it up with search results.
fork-maintainers · Sep 30, 2021 · 0d9e2b3 · 0d9e2b3
1 parent 38b6736
commit 0d9e2b3
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 2 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt
@@ -92,7 +92,11 @@ class DefaultBrowserToolbarController(
 
     override fun handleToolbarClick() {
         metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER))
-        if (FeatureFlags.showHomeBehindSearch) {
+        // 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
+        // results on top it.
+        if (FeatureFlags.showHomeBehindSearch && currentSession?.content?.searchTerms.isNullOrBlank()) {
             navController.navigate(
                 BrowserFragmentDirections.actionGlobalHome()
             )

diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
@@ -434,7 +434,10 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
                 true
             }
             else -> {
-                if (FeatureFlags.showHomeBehindSearch) {
+                // In case we're displaying search results, we wouldn't have navigated to home, and
+                // so we don't need to navigate "back to" browser fragment.
+                // See mirror of this logic in BrowserToolbarController#handleToolbarClick.
+                if (FeatureFlags.showHomeBehindSearch && store.state.searchTerms.isBlank()) {
                     val args by navArgs<SearchDialogFragmentArgs>()
                     args.sessionId?.let {
                         findNavController().navigate(

diff --git a/...src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/...src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt
@@ -238,6 +238,31 @@ class DefaultBrowserToolbarControllerTest {
         }
     }
 
+    @Test
+    fun handleToolbackClickWithSearchTerms() {
+        val searchResultsTab = createTab("https://google.com?q=mozilla+website", searchTerms = "mozilla website")
+        store.dispatch(TabListAction.AddTabAction(searchResultsTab, select = true)).joinBlocking()
+
+        val controller = createController()
+        controller.handleToolbarClick()
+
+        val homeDirections = BrowserFragmentDirections.actionGlobalHome()
+        val searchDialogDirections = BrowserFragmentDirections.actionGlobalSearchDialog(
+            sessionId = searchResultsTab.id
+        )
+
+        verify {
+            metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER))
+        }
+        // Does not show the home screen "behind" the search dialog if the current session has search terms.
+        verify(exactly = 0) {
+            navController.navigate(homeDirections)
+        }
+        verify {
+            navController.navigate(searchDialogDirections, any<NavOptions>())
+        }
+    }
+
     @Test
     fun handleToolbarCloseTabPressWithLastPrivateSession() {
         val item = TabCounterMenu.Item.CloseTab