Close #12398: Do not scroll menu to bottom with up orientation

mozilla-mobile · Jun 29, 2022 · 19fa535 · 19fa535
1 parent b52a7aa
commit 19fa535
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 13 deletions.
diff --git a/...nts/browser/menu2/src/main/java/mozilla/components/browser/menu2/BrowserMenuController.kt b/...nts/browser/menu2/src/main/java/mozilla/components/browser/menu2/BrowserMenuController.kt
@@ -60,10 +60,6 @@ class BrowserMenuController(
             view.onReopenMenu = ::reopenMenu
             setOnDismissListener(menuDismissListener)
             displayPopup(view, anchor, orientation, forceOrientation)
-
-            if (orientation == Orientation.UP && forceOrientation) {
-                view.scrollOnceToTheBottom()
-            }
         }.also {
             currentPopupInfo = PopupMenuInfo(
                 window = it,

diff --git a/components/browser/menu2/src/main/java/mozilla/components/browser/menu2/view/MenuView.kt b/components/browser/menu2/src/main/java/mozilla/components/browser/menu2/view/MenuView.kt
@@ -11,6 +11,7 @@ import android.util.AttributeSet
 import android.view.LayoutInflater
 import android.view.View
 import android.widget.FrameLayout
+import androidx.annotation.VisibleForTesting
 import androidx.cardview.widget.CardView
 import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
@@ -74,7 +75,7 @@ class MenuView @JvmOverloads constructor(
             // In devices with Android 6 and below stackFromEnd is not working properly,
             // as a result, we have to provided a backwards support.
             // See: https://github.com/mozilla-mobile/android-components/issues/3211
-            if (side == Side.END) scrollOnceToTheBottom()
+            if (side == Side.END) scrollOnceToTheBottom(recyclerView)
         }
     }
 
@@ -85,10 +86,8 @@ class MenuView @JvmOverloads constructor(
         style.backgroundColor?.let { cardView.setCardBackgroundColor(it) }
     }
 
-    /**
-     * Scroll to the bottom of the menu view.
-     */
-    fun scrollOnceToTheBottom() {
+    @VisibleForTesting
+    internal fun scrollOnceToTheBottom(recyclerView: RecyclerView) {
         recyclerView.onNextGlobalLayout {
             recyclerView.adapter?.let { recyclerView.scrollToPosition(it.itemCount - 1) }
         }

diff --git a/components/browser/menu2/src/test/java/mozilla/components/browser/menu2/view/MenuViewTest.kt b/components/browser/menu2/src/test/java/mozilla/components/browser/menu2/view/MenuViewTest.kt
@@ -15,6 +15,7 @@ import mozilla.components.browser.menu2.R
 import mozilla.components.concept.menu.MenuStyle
 import mozilla.components.concept.menu.Side
 import mozilla.components.concept.menu.candidate.DecorativeTextMenuCandidate
+import mozilla.components.support.test.any
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
@@ -62,25 +63,25 @@ class MenuViewTest {
     @Test
     @Config(sdk = [Build.VERSION_CODES.M])
     fun `setVisibleSide will be forwarded to scrollOnceToTheBottom on devices with Android M and below`() {
-        doNothing().`when`(menuView).scrollOnceToTheBottom()
+        doNothing().`when`(menuView).scrollOnceToTheBottom(any())
 
         menuView.setVisibleSide(Side.END)
         val layoutManager = recyclerView.layoutManager as LinearLayoutManager
 
         assertFalse(layoutManager.stackFromEnd)
-        verify(menuView).scrollOnceToTheBottom()
+        verify(menuView).scrollOnceToTheBottom(any())
     }
 
     @Test
     @Config(sdk = [Build.VERSION_CODES.N])
     fun `setVisibleSide changes stackFromEnd on devices with Android N and above`() {
-        doNothing().`when`(menuView).scrollOnceToTheBottom()
+        doNothing().`when`(menuView).scrollOnceToTheBottom(any())
 
         menuView.setVisibleSide(Side.END)
         val layoutManager = recyclerView.layoutManager as LinearLayoutManager
 
         assertTrue(layoutManager.stackFromEnd)
-        verify(menuView, never()).scrollOnceToTheBottom()
+        verify(menuView, never()).scrollOnceToTheBottom(any())
     }
 
     @Test