For mozilla-mobile#26957 - Exit search dialog when interacting with h…

…ome fragment (mozilla-mobile#27262) * Revert "For mozilla-mobile#26790 - Dismiss search dialog when opening recent bookmark dropdown menu" This reverts commit 262aa16. * Revert "For mozilla-mobile#26790 - Dismiss search dialog when opening recent visit dropdown menu" This reverts commit b93b085 * Revert "For mozilla-mobile#26790 - Dismiss search dialog when opening recent tab dropdown menu" This reverts commit 44b71bb. * Revert "For mozilla-mobile#26690 - Dismiss search dialog when opening recent synced tab dropdown menu" This reverts commit bda817a. * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with homescreen top sites * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with homescreen collection * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with homescreen recent visits * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with homescreen recent tabs * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with homescreen recent bookmarks * For mozilla-mobile#26957 - Remove code to dismiss search dialog when interacting with pocket stories * For mozilla-mobile#26957 - Dismiss search dialog when interacting with home fragment Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AndiAJ · Dec 13, 2022 · 4ee62dd · 4ee62dd
1 parent df4e08f
commit 4ee62dd
Show file tree

Hide file tree

Showing 39 changed files with 87 additions and 483 deletions.
diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt
@@ -390,6 +390,7 @@ class SettingsSearchTest {
 
     // Expected for app language set to Arabic
     @Test
+    @Ignore("Failing after changing SearchDialog homescreen interaction. See: https://github.com/mozilla-mobile/fenix/issues/28182")
     fun verifySearchEnginesWithRTLLocale() {
         homeScreen {
         }.openThreeDotMenu {
@@ -430,6 +431,7 @@ class SettingsSearchTest {
 
     // Expected for en-us defaults
     @Test
+    @Ignore("Failing after changing SearchDialog homescreen interaction. See: https://github.com/mozilla-mobile/fenix/issues/28182")
     fun toggleSearchEnginesShortcutListTest() {
         homeScreen {
         }.openThreeDotMenu {

diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt
@@ -417,7 +417,6 @@ class HomeFragment : Fragment() {
             pocketStoriesController = DefaultPocketStoriesController(
                 homeActivity = activity,
                 appStore = components.appStore,
-                navController = findNavController(),
             ),
         )
 

diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragmentLayout.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragmentLayout.kt
@@ -0,0 +1,57 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.home
+
+import android.content.Context
+import android.graphics.Rect
+import android.util.AttributeSet
+import android.view.MotionEvent
+import android.view.MotionEvent.ACTION_UP
+import androidx.coordinatorlayout.widget.CoordinatorLayout
+import androidx.navigation.findNavController
+import mozilla.components.support.ktx.android.view.findViewInHierarchy
+import org.mozilla.fenix.R
+import org.mozilla.fenix.search.SearchDialogFragment
+
+/**
+ * Parent layout for [HomeFragment], used to dismiss the [SearchDialogFragment] when
+ * interacting with elements in the [HomeFragment].
+ */
+class HomeFragmentLayout @JvmOverloads constructor(
+    context: Context,
+    attrs: AttributeSet? = null,
+    defStyleAttr: Int = 0,
+) : CoordinatorLayout(context, attrs, defStyleAttr) {
+
+    /**
+     * Returns whether or not the motion event is touching the private browsing button.
+     *
+     * @param x X coordinate of the event.
+     * @param y Y coordinate of the event.
+     */
+    private fun isTouchingPrivateButton(x: Float, y: Float): Boolean {
+        val view = this.findViewInHierarchy {
+            it.id == R.id.privateBrowsingButton
+        } ?: return false
+        val privateButtonRect = Rect()
+        view.getHitRect(privateButtonRect)
+        return privateButtonRect.contains(x.toInt(), y.toInt())
+    }
+
+    override fun onInterceptTouchEvent(ev: MotionEvent?): Boolean {
+        val nav = findNavController()
+
+        // If the private button is touched from the [SearchDialogFragment], then it should not be
+        // dismissed to allow the user to continue the search.
+        if (ev?.action == ACTION_UP &&
+            nav.currentDestination?.id == R.id.searchDialogFragment &&
+            !isTouchingPrivateButton(ev.x, ev.y)
+        ) {
+            nav.popBackStack()
+        }
+
+        return super.onInterceptTouchEvent(ev)
+    }
+}
diff --git a/app/src/main/java/org/mozilla/fenix/home/collections/Collection.kt b/app/src/main/java/org/mozilla/fenix/home/collections/Collection.kt
@@ -62,17 +62,15 @@ private val expandedCollectionShape = RoundedCornerShape(topStart = 8.dp, topEnd
  * @param menuItems List of [CollectionMenuItem] to be shown in a menu.
  * @param onToggleCollectionExpanded Invoked when the user clicks on the collection.
  * @param onCollectionShareTabsClicked Invoked when the user clicks to share the collection.
- * @param onCollectionMenuOpened Invoked when the user clicks to open a menu for the collection.
  */
 @Composable
-@Suppress("LongParameterList", "LongMethod")
+@Suppress("LongMethod")
 fun Collection(
     collection: TabCollection,
     expanded: Boolean,
     menuItems: List<CollectionMenuItem>,
     onToggleCollectionExpanded: (TabCollection, Boolean) -> Unit,
     onCollectionShareTabsClicked: (TabCollection) -> Unit,
-    onCollectionMenuOpened: () -> Unit,
 ) {
     var isMenuExpanded by remember(collection) { mutableStateOf(false) }
     val isExpanded by remember(collection) { mutableStateOf(expanded) }
@@ -131,7 +129,6 @@ fun Collection(
                         IconButton(
                             onClick = {
                                 isMenuExpanded = !isMenuExpanded
-                                onCollectionMenuOpened()
                             },
                         ) {
                             Icon(
@@ -165,7 +162,6 @@ private fun CollectionDarkPreview() {
             menuItems = emptyList(),
             onToggleCollectionExpanded = { _, _ -> },
             onCollectionShareTabsClicked = {},
-            onCollectionMenuOpened = {},
         )
     }
 }
@@ -180,7 +176,6 @@ private fun CollectionDarkExpandedPreview() {
             menuItems = emptyList(),
             onToggleCollectionExpanded = { _, _ -> },
             onCollectionShareTabsClicked = {},
-            onCollectionMenuOpened = {},
         )
     }
 }
@@ -195,7 +190,6 @@ private fun CollectionLightPreview() {
             menuItems = emptyList(),
             onToggleCollectionExpanded = { _, _ -> },
             onCollectionShareTabsClicked = {},
-            onCollectionMenuOpened = {},
         )
     }
 }
@@ -210,7 +204,6 @@ private fun CollectionLightExpandedPreview() {
             menuItems = emptyList(),
             onToggleCollectionExpanded = { _, _ -> },
             onCollectionShareTabsClicked = {},
-            onCollectionMenuOpened = {},
         )
     }
 }

diff --git a/app/src/main/java/org/mozilla/fenix/home/collections/CollectionViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/collections/CollectionViewHolder.kt
@@ -67,7 +67,6 @@ class CollectionViewHolder(
                     menuItems = menuItems,
                     onToggleCollectionExpanded = interactor::onToggleCollectionExpanded,
                     onCollectionShareTabsClicked = interactor::onCollectionShareTabsClicked,
-                    onCollectionMenuOpened = interactor::onCollectionMenuOpened,
                 )
             }
         }

diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt
@@ -4,8 +4,6 @@
 
 package org.mozilla.fenix.home.pocket
 
-import androidx.annotation.VisibleForTesting
-import androidx.navigation.NavController
 import mozilla.components.service.glean.private.NoExtras
 import mozilla.components.service.pocket.PocketStory
 import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory
@@ -15,7 +13,6 @@ import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.Pings
 import org.mozilla.fenix.GleanMetrics.Pocket
 import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.R
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction
 
@@ -73,12 +70,10 @@ interface PocketStoriesController {
  *
  * @param homeActivity [HomeActivity] used to open URLs in a new tab.
  * @param appStore [AppStore] from which to read the current Pocket recommendations and dispatch new actions on.
- * @param navController [NavController] used for navigation.
  */
 internal class DefaultPocketStoriesController(
     private val homeActivity: HomeActivity,
     private val appStore: AppStore,
-    private val navController: NavController,
 ) : PocketStoriesController {
     override fun handleStoryShown(
         storyShown: PocketStory,
@@ -153,7 +148,6 @@ internal class DefaultPocketStoriesController(
         storyClicked: PocketStory,
         storyPosition: Pair<Int, Int>,
     ) {
-        dismissSearchDialogIfDisplayed()
         homeActivity.openToBrowserAndLoad(storyClicked.url, true, BrowserDirection.FromHome)
 
         when (storyClicked) {
@@ -179,21 +173,12 @@ internal class DefaultPocketStoriesController(
     }
 
     override fun handleLearnMoreClicked(link: String) {
-        dismissSearchDialogIfDisplayed()
         homeActivity.openToBrowserAndLoad(link, true, BrowserDirection.FromHome)
         Pocket.homeRecsLearnMoreClicked.record(NoExtras())
     }
 
     override fun handleDiscoverMoreClicked(link: String) {
-        dismissSearchDialogIfDisplayed()
         homeActivity.openToBrowserAndLoad(link, true, BrowserDirection.FromHome)
         Pocket.homeRecsDiscoverClicked.record(NoExtras())
     }
-
-    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    internal fun dismissSearchDialogIfDisplayed() {
-        if (navController.currentDestination?.id == R.id.searchDialogFragment) {
-            navController.navigateUp()
-        }
-    }
 }
diff --git a/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt b/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt
@@ -4,16 +4,13 @@
 
 package org.mozilla.fenix.home.recentbookmarks.controller
 
-import androidx.annotation.VisibleForTesting
-import androidx.annotation.VisibleForTesting.Companion.PRIVATE
 import androidx.navigation.NavController
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.R
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction
 import org.mozilla.fenix.home.HomeFragmentDirections
@@ -40,11 +37,6 @@ interface RecentBookmarksController {
      * @see [RecentBookmarksInteractor.onRecentBookmarkRemoved]
      */
     fun handleBookmarkRemoved(bookmark: RecentBookmark)
-
-    /**
-     * @see [RecentBookmarksInteractor.onRecentBookmarkLongClicked]
-     */
-    fun handleBookmarkLongClicked()
 }
 
 /**
@@ -57,7 +49,6 @@ class DefaultRecentBookmarksController(
 ) : RecentBookmarksController {
 
     override fun handleBookmarkClicked(bookmark: RecentBookmark) {
-        dismissSearchDialogIfDisplayed()
         activity.openToBrowserAndLoad(
             searchTermOrURL = bookmark.url!!,
             newTab = true,
@@ -69,7 +60,6 @@ class DefaultRecentBookmarksController(
 
     override fun handleShowAllBookmarksClicked() {
         RecentBookmarks.showAllBookmarks.add()
-        dismissSearchDialogIfDisplayed()
         navController.navigate(
             HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id),
         )
@@ -78,15 +68,4 @@ class DefaultRecentBookmarksController(
     override fun handleBookmarkRemoved(bookmark: RecentBookmark) {
         appStore.dispatch(AppAction.RemoveRecentBookmark(bookmark))
     }
-
-    override fun handleBookmarkLongClicked() {
-        dismissSearchDialogIfDisplayed()
-    }
-
-    @VisibleForTesting(otherwise = PRIVATE)
-    fun dismissSearchDialogIfDisplayed() {
-        if (navController.currentDestination?.id == R.id.searchDialogFragment) {
-            navController.navigateUp()
-        }
-    }
 }
diff --git a/.../main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt b/.../main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt
@@ -33,9 +33,4 @@ interface RecentBookmarksInteractor {
      * @param bookmark The bookmark that has been removed.
      */
     fun onRecentBookmarkRemoved(bookmark: RecentBookmark)
-
-    /**
-     * Called when the user long clicks a recent bookmark.
-     */
-    fun onRecentBookmarkLongClicked()
 }
diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt
@@ -70,7 +70,6 @@ private val imageModifier = Modifier
  * @param menuItems List of [RecentBookmarksMenuItem] shown when long clicking a [RecentBookmarkItem]
  * @param backgroundColor The background [Color] of each bookmark.
  * @param onRecentBookmarkClick Invoked when the user clicks on a recent bookmark.
- * @param onRecentBookmarkLongClick Invoked when the user long clicks on a recent bookmark.
  */
 @OptIn(ExperimentalComposeUiApi::class)
 @Composable
@@ -79,7 +78,6 @@ fun RecentBookmarks(
     menuItems: List<RecentBookmarksMenuItem>,
     backgroundColor: Color,
     onRecentBookmarkClick: (RecentBookmark) -> Unit = {},
-    onRecentBookmarkLongClick: () -> Unit = {},
 ) {
     LazyRow(
         modifier = Modifier.semantics {
@@ -95,7 +93,6 @@ fun RecentBookmarks(
                 menuItems = menuItems,
                 backgroundColor = backgroundColor,
                 onRecentBookmarkClick = onRecentBookmarkClick,
-                onRecentBookmarkLongClick = onRecentBookmarkLongClick,
             )
         }
     }
@@ -108,7 +105,6 @@ fun RecentBookmarks(
  * @param menuItems The list of [RecentBookmarksMenuItem] shown when long clicking on the recent bookmark item.
  * @param backgroundColor The background [Color] of the recent bookmark item.
  * @param onRecentBookmarkClick Invoked when the user clicks on the recent bookmark item.
- * @param onRecentBookmarkLongClick Invoked when the user long clicks on the recent bookmark item.
  */
 @OptIn(
     ExperimentalFoundationApi::class,
@@ -120,7 +116,6 @@ private fun RecentBookmarkItem(
     menuItems: List<RecentBookmarksMenuItem>,
     backgroundColor: Color,
     onRecentBookmarkClick: (RecentBookmark) -> Unit = {},
-    onRecentBookmarkLongClick: () -> Unit = {},
 ) {
     var isMenuExpanded by remember { mutableStateOf(false) }
 
@@ -130,10 +125,7 @@ private fun RecentBookmarkItem(
             .combinedClickable(
                 enabled = true,
                 onClick = { onRecentBookmarkClick(bookmark) },
-                onLongClick = {
-                    onRecentBookmarkLongClick()
-                    isMenuExpanded = true
-                },
+                onLongClick = { isMenuExpanded = true },
             ),
         shape = cardShape,
         backgroundColor = backgroundColor,

diff --git a/.../main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksHeaderViewHolder.kt b/.../main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksHeaderViewHolder.kt
@@ -14,7 +14,6 @@ import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
 import androidx.lifecycle.LifecycleOwner
-import androidx.navigation.findNavController
 import org.mozilla.fenix.R
 import org.mozilla.fenix.compose.ComposeViewHolder
 import org.mozilla.fenix.compose.home.HomeSectionHeader
@@ -38,13 +37,6 @@ class RecentBookmarksHeaderViewHolder(
         composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0)
     }
 
-    private fun dismissSearchDialogIfDisplayed() {
-        val navController = itemView.findNavController()
-        if (navController.currentDestination?.id == R.id.searchDialogFragment) {
-            navController.navigateUp()
-        }
-    }
-
     @Composable
     override fun Content() {
         Column {
@@ -54,7 +46,6 @@ class RecentBookmarksHeaderViewHolder(
                 headerText = stringResource(R.string.recently_saved_title),
                 description = stringResource(R.string.recently_saved_show_all_content_description_2),
                 onShowAllClick = {
-                    dismissSearchDialogIfDisplayed()
                     interactor.onShowAllBookmarksClicked()
                 },
             )

diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksViewHolder.kt
@@ -48,7 +48,6 @@ class RecentBookmarksViewHolder(
                     onClick = { bookmark -> interactor.onRecentBookmarkRemoved(bookmark) },
                 ),
             ),
-            onRecentBookmarkLongClick = interactor::onRecentBookmarkLongClicked,
         )
     }
 }
diff --git a/...main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt b/...main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt
@@ -25,11 +25,6 @@ interface RecentSyncedTabController {
      */
     fun handleRecentSyncedTabClick(tab: RecentSyncedTab)
 
-    /**
-     * @see [RecentSyncedTabInteractor.onRecentSyncedTabLongClick]
-     */
-    fun handleRecentSyncedTabLongClick()
-
     /**
      * @see [RecentSyncedTabInteractor.onRecentSyncedTabClicked]
      */
@@ -71,12 +66,6 @@ class DefaultRecentSyncedTabController(
         )
     }
 
-    override fun handleRecentSyncedTabLongClick() {
-        if (navController.currentDestination?.id == R.id.searchDialogFragment) {
-            navController.navigateUp()
-        }
-    }
-
     override fun handleRecentSyncedTabRemoved(tab: RecentSyncedTab) {
         appStore.dispatch(AppAction.RemoveRecentSyncedTab(tab))
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -48,7 +48,6 @@ class RecentBookmarksViewHolder( @@
                         onClick = { bookmark -> interactor.onRecentBookmarkRemoved(bookmark) },
                     ),
                 ),
-                onRecentBookmarkLongClick = interactor::onRecentBookmarkLongClicked,
             )
         }
     }