For mozilla-mobile#10005 - Replace layout manager to effectively chan…

…ge stackFromEnd

The `stackFromEnd` property is not initially set even though
`endOfMenuAlwaysVisible` might be true since when having a bottom expandable
menu, that should always be scrolled to the top.
And we know if we're in that situation only later, after menu is configured and
based on it it can be inferred if it should be collapsed or not.

Setting `stackFromEnd` only after this steps, even if the menu is not yet on
the screen seems to have no effect.

The only solution is to replace menu's layout manager with one that has this
`stackFromEnd` from the beginning.

components/browser/menu/src/main/java/mozilla/components/browser/menu/BrowserMenu.kt

@@ -35,7 +35,8 @@ open class BrowserMenu internal constructor(
     internal val adapter: BrowserMenuAdapter
 ) : View.OnAttachStateChangeListener {
     protected var currentPopup: PopupWindow? = null
-    private var menuList: RecyclerView? = null
+    @VisibleForTesting
+    internal var menuList: RecyclerView? = null
     internal var currAnchor: View? = null
     internal var isShown = false
     @VisibleForTesting
@@ -133,14 +134,22 @@ open class BrowserMenu internal constructor(
 
         // When showing an expandable bottom menu it should always be scrolled to the top (default in LayoutManager).
         // Otherwise try showing the bottom of the menu when not enough space to fit it on the screen.
-        menuList?.let { list ->
-            list.setEndOfMenuAlwaysVisibleCompact(
-                endOfMenuAlwaysVisible, list.layoutManager as LinearLayoutManager
-            )
+        if (endOfMenuAlwaysVisible) {
+            menuList?.let {
+                showMenuBottom(it)
+            }
         }
+
         return view
     }
 
+    @VisibleForTesting
+    internal fun showMenuBottom(menu: RecyclerView) {
+        menu.layoutManager = LinearLayoutManager(menu.context, RecyclerView.VERTICAL, false).also {
+            menu.setEndOfMenuAlwaysVisibleCompact(true, it)
+        }
+    }
+
     @VisibleForTesting
     internal fun getNewPopupWindow(view: ViewGroup): PopupWindow {
         // If the menu is expandable we need to give it all the possible space to expand.

components/browser/menu/src/test/java/mozilla/components/browser/menu/BrowserMenuTest.kt

@@ -9,6 +9,7 @@ import android.graphics.Color
 import android.os.Build
 import android.view.Gravity
 import android.view.View
+import android.view.ViewGroup
 import android.view.ViewGroup.LayoutParams.MATCH_PARENT
 import android.view.ViewGroup.LayoutParams.WRAP_CONTENT
 import android.widget.Button
@@ -29,12 +30,14 @@ import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNotSame
 import org.junit.Assert.assertSame
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito
 import org.mockito.Mockito.doNothing
+import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.verify
 import org.robolectric.Shadows
@@ -353,6 +356,44 @@ class BrowserMenuTest {
         assertTrue(result == view)
     }
 
+    @Test
+    fun `GIVEN a not expandable menu WHEN configureExpandableMenu is called for one which should not be scrolled to bottom THEN the same menu is returned`() {
+        val menu = spy(BrowserMenu(mock()))
+        menu.menuPositioningData = MenuPositioningData(BrowserMenuPlacement.AnchoredToTop.Dropdown(mock()))
+        val viewGroup: ViewGroup = mock()
+
+        val result = menu.configureExpandableMenu(viewGroup, false)
+
+        assertSame(viewGroup, result)
+        verify(menu, never()).showMenuBottom(any())
+    }
+
+    @Test
+    fun `GIVEN a not expandable menu WHEN configureExpandableMenu is called for one which should be scrolled to bottom THEN the layout manager is updated for this`() {
+        val menu = spy(BrowserMenu(mock()))
+        menu.menuPositioningData = MenuPositioningData(BrowserMenuPlacement.AnchoredToTop.Dropdown(mock()))
+        val menuList = RecyclerView(testContext)
+        menu.menuList = menuList
+
+        val result = menu.configureExpandableMenu(menuList, true)
+
+        assertSame(menuList, result)
+        verify(menu).showMenuBottom(menuList)
+    }
+
+    @Test
+    fun `GIVEN a menu that should be scrolled to the bottom WHEN showMenuBottom is called THEN it replaces the layout manager and sets stackFromEnd`() {
+        val menu = spy(BrowserMenu(mock()))
+        // Call show to have a default layout manager set
+        menu.show(View(testContext))
+        val initialLayoutManager = menu.menuList!!.layoutManager
+
+        menu.showMenuBottom(menu.menuList!!)
+
+        assertNotSame(initialLayoutManager, menu.menuList!!.layoutManager)
+        assertTrue((menu.menuList!!.layoutManager as LinearLayoutManager).stackFromEnd)
+    }
+
     @Test
     fun `getNewPopupWindow will return a PopupWindow with MATCH_PARENT height if the view is ExpandableLayout`() {
         val expandableLayout = ExpandableLayout.wrapContentInExpandableView(FrameLayout(testContext), 0) { }

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/10005) - Fix a recent issue with BrowserMenu#show() - endOfMenuAlwaysVisible not being applied.
+
 * **browser-menu**:
   * 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/9922) - The browser menu will have it's dynamic width calculated only once, before the first layout.