For mozilla-mobile#10032 - Handle menu touches in expandable menu on …

…Samsung devices Refactored the code so that touches/scrolls are still swallowed while the menu is expanding after which ExpandableLayout should let all MotionEvents pass through to be handled by menu items children.
Mugurell · Apr 6, 2021 · 7811c60 · 7811c60
1 parent 34f557d
commit 7811c60
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 37 deletions.
diff --git a/...nents/browser/menu/src/main/java/mozilla/components/browser/menu/view/ExpandableLayout.kt b/...nents/browser/menu/src/main/java/mozilla/components/browser/menu/view/ExpandableLayout.kt
@@ -140,10 +140,6 @@ internal class ExpandableLayout private constructor(context: Context) : FrameLay
                     false // Allow click listeners firing for children.
                 }
                 MotionEvent.ACTION_MOVE -> {
-                    if (isExpandInProgress || !isCollapsed) {
-                        return true
-                    }
-
                     if (isScrollingUp(ev)) {
                         expand()
                         true
@@ -157,13 +153,23 @@ internal class ExpandableLayout private constructor(context: Context) : FrameLay
                     return callParentOnInterceptTouchEvent(ev)
                 }
             }
+        } else {
+            return if (ev != null && !isTouchingTheWrappedView(ev)) {
+                // If the menu is expanded but smaller than the parent height
+                // and the user touches above the menu, in the empty space.
+                blankTouchListener?.invoke()
+                true
+            } else if (isExpandInProgress) {
+                // Swallow all menu touches while the menu is expanding.
+                true
+            } else {
+                callParentOnInterceptTouchEvent(ev)
+            }
         }
-
-        return callParentOnInterceptTouchEvent(ev)
     }
 
     @VisibleForTesting
-    internal fun shouldInterceptTouches() = isCollapsed || parentHeight > expandedHeight
+    internal fun shouldInterceptTouches() = isCollapsed && !isExpandInProgress
 
     @VisibleForTesting
     internal fun isTouchingTheWrappedView(ev: MotionEvent): Boolean {

diff --git a/...s/browser/menu/src/test/java/mozilla/components/browser/menu/view/ExpandableLayoutTest.kt b/...s/browser/menu/src/test/java/mozilla/components/browser/menu/view/ExpandableLayoutTest.kt
@@ -142,18 +142,54 @@ class ExpandableLayoutTest {
     }
 
     @Test
-    fun `GIVEN ExpandableLayout WHEN onInterceptTouchEvent is called for an event for which should not be intercepted THEN super is called`() {
+    fun `GIVEN an expanded menu WHEN onInterceptTouchEvent is called for a touch on the menu THEN super is called`() {
+        val blankTouchListener = spy {}
         val expandableLayout = spy(ExpandableLayout.wrapContentInExpandableView(
-            FrameLayout(testContext), 1) { }
+            FrameLayout(testContext), 1, blankTouchListener)
         )
         val event: MotionEvent = mock()
         doReturn(false).`when`(expandableLayout).shouldInterceptTouches()
+        doReturn(true).`when`(expandableLayout).isTouchingTheWrappedView(any())
 
         expandableLayout.onInterceptTouchEvent(event)
 
+        verify(blankTouchListener, never()).invoke()
         verify(expandableLayout).callParentOnInterceptTouchEvent(event)
     }
 
+    @Test
+    fun `GIVEN a menu currently expanding WHEN onInterceptTouchEvent is called for a touch on the menu THEN the touch is swallowed`() {
+        val blankTouchListener = spy {}
+        val expandableLayout = spy(ExpandableLayout.wrapContentInExpandableView(
+            FrameLayout(testContext), 1, blankTouchListener)
+        )
+        val event: MotionEvent = mock()
+        doReturn(false).`when`(expandableLayout).shouldInterceptTouches()
+        doReturn(true).`when`(expandableLayout).isTouchingTheWrappedView(any())
+        expandableLayout.isExpandInProgress = true
+
+        expandableLayout.onInterceptTouchEvent(event)
+
+        verify(blankTouchListener, never()).invoke()
+        verify(expandableLayout, never()).callParentOnInterceptTouchEvent(event)
+    }
+
+    @Test
+    fun `GIVEN an expanded menu WHEN onInterceptTouchEvent is called for a outside the menu THEN super blankTouchListener is invoked`() {
+        val blankTouchListener = spy {}
+        val expandableLayout = spy(ExpandableLayout.wrapContentInExpandableView(
+            FrameLayout(testContext), 1, blankTouchListener)
+        )
+        val event: MotionEvent = mock()
+        doReturn(false).`when`(expandableLayout).shouldInterceptTouches()
+        doReturn(false).`when`(expandableLayout).isTouchingTheWrappedView(any())
+
+        expandableLayout.onInterceptTouchEvent(event)
+
+        verify(blankTouchListener).invoke()
+        verify(expandableLayout, never()).callParentOnInterceptTouchEvent(event)
+    }
+
     @Test
     fun `GIVEN ExpandableLayout WHEN onInterceptTouchEvent is called for ACTION_CANCEL or ACTION_UP THEN the events are not intercepted`() {
         val expandableLayout = spy(ExpandableLayout.wrapContentInExpandableView(
@@ -246,23 +282,8 @@ class ExpandableLayoutTest {
             FrameLayout(testContext), 1) { }
         )
         val actionDown = MotionEvent.obtain(0, 0, ACTION_MOVE, 0f, 0f, 0)
-        doReturn(true).`when`(expandableLayout).shouldInterceptTouches()
-        expandableLayout.isExpandInProgress = true
-
-        assertTrue(expandableLayout.onInterceptTouchEvent(actionDown))
-        verify(expandableLayout, never()).expand()
-        verify(expandableLayout, never()).callParentOnInterceptTouchEvent(any())
-    }
-
-    @Test
-    fun `GIVEN the wrappedView is expanded but smaller than the parent WHEN onInterceptTouchEvent is called for scroll events THEN these events are intercepted`() {
-        val expandableLayout = spy(ExpandableLayout.wrapContentInExpandableView(
-            FrameLayout(testContext), 1) { }
-        )
-        val actionDown = MotionEvent.obtain(0, 0, ACTION_MOVE, 0f, 0f, 0)
-        doReturn(true).`when`(expandableLayout).shouldInterceptTouches()
-        expandableLayout.isExpandInProgress = false
-        expandableLayout.isCollapsed = false
+        doReturn(false).`when`(expandableLayout).shouldInterceptTouches()
+        doReturn(false).`when`(expandableLayout).isTouchingTheWrappedView(any())
 
         assertTrue(expandableLayout.onInterceptTouchEvent(actionDown))
         verify(expandableLayout, never()).expand()
@@ -277,7 +298,6 @@ class ExpandableLayoutTest {
         val actionDown = MotionEvent.obtain(0, 0, ACTION_MOVE, 0f, 0f, 0)
         doReturn(true).`when`(expandableLayout).shouldInterceptTouches()
         doReturn(false).`when`(expandableLayout).isScrollingUp(any())
-        expandableLayout.isExpandInProgress = false
 
         assertFalse(expandableLayout.onInterceptTouchEvent(actionDown))
         verify(expandableLayout, never()).expand()
@@ -292,7 +312,6 @@ class ExpandableLayoutTest {
         val actionDown = MotionEvent.obtain(0, 0, ACTION_MOVE, 0f, 0f, 0)
         doReturn(true).`when`(expandableLayout).shouldInterceptTouches()
         doReturn(true).`when`(expandableLayout).isScrollingUp(any())
-        expandableLayout.isExpandInProgress = false
 
         assertTrue(expandableLayout.onInterceptTouchEvent(actionDown))
         verify(expandableLayout).expand()
@@ -340,27 +359,24 @@ class ExpandableLayoutTest {
     }
 
     @Test
-    fun `GIVEN ExpandableLayout not collapsed and the wrappedView smaller than it's parent WHEN shouldInterceptTouches is called THEN it returns true`() {
+    fun `GIVEN ExpandableLayout not collapsed WHEN shouldInterceptTouches is called THEN it returns false`() {
         val expandableLayout = ExpandableLayout.wrapContentInExpandableView(
             FrameLayout(testContext), 1
         ) { }
         expandableLayout.isCollapsed = false
-        expandableLayout.parentHeight = 101
-        expandableLayout.expandedHeight = 100
 
-        assertTrue(expandableLayout.shouldInterceptTouches())
+        assertFalse(expandableLayout.shouldInterceptTouches())
     }
 
     @Test
-    fun `GIVEN ExpandableLayout not collapsed and the wrappedView bigger than it's parent WHEN shouldInterceptTouches is called THEN it returns false`() {
+    fun `GIVEN ExpandableLayout currently expanding WHEN shouldInterceptTouches is called THEN it returns false`() {
         val expandableLayout = ExpandableLayout.wrapContentInExpandableView(
             FrameLayout(testContext), 1
         ) { }
-        expandableLayout.isCollapsed = false
-        expandableLayout.parentHeight = 101
-        expandableLayout.expandedHeight = 102
+        expandableLayout.isCollapsed = true
+        expandableLayout.isExpandInProgress = false
 
-        assertFalse(expandableLayout.shouldInterceptTouches())
+        assertTrue(expandableLayout.shouldInterceptTouches())
     }
 
     @Test

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -12,6 +12,9 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml)
 
+* **browser-menu**:
+  * 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/10032) - Fix a recent issue with ExpandableLayout - user touches on an expanded menu might not have any effect on Samsung devices.
+
 * **browser-menu**:
   * 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/10005) - Fix a recent issue with BrowserMenu#show() - endOfMenuAlwaysVisible not being applied.