For mozilla-mobile#10133 - BrowserMenuCompoundButton now reports the …

…right dimensions

A BrowserMenuCompoundButton setup with compound drawables and used in our
BrowserMenu configured with a DynamicWidthRecyclerVIew will return a width
smaller with the size + padding of the compound drawables.

By replacing the default `inherit` layout direction for the initial onMeasure
we ensure the menu will also count the bounds of the compound drawables.

components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButton.kt

@@ -6,6 +6,7 @@ package mozilla.components.browser.menu.item
 
 import android.content.Context
 import android.view.View
+import android.view.ViewTreeObserver
 import android.widget.CompoundButton
 import androidx.annotation.VisibleForTesting
 import mozilla.components.browser.menu.BrowserMenu
@@ -36,6 +37,23 @@ abstract class BrowserMenuCompoundButton(
     override var visible: () -> Boolean = { true }
 
     override fun bind(menu: BrowserMenu, view: View) {
+        // A CompoundButton containing CompoundDrawables needs to know where to place them (LTR / RTL).
+        // If the View is not yet attached to Window the direction inference will fail and the menu item
+        // will return from it's onMeasure a width smaller with the size + padding of the compound drawables.
+        // Work around this by setting a valid layout direction and reset it to inherit from parent later.
+        if (!view.isAttachedToWindow) {
+            view.layoutDirection = View.LAYOUT_DIRECTION_LOCALE
+
+            view.viewTreeObserver.addOnPreDrawListener(
+                object : ViewTreeObserver.OnPreDrawListener {
+                    override fun onPreDraw(): Boolean {
+                        view.viewTreeObserver.removeOnPreDrawListener(this)
+                        view.layoutDirection = View.LAYOUT_DIRECTION_INHERIT
+                        return true
+                    }
+                })
+        }
+
         (view as CompoundButton).apply {
             text = label
             isChecked = initialState()

components/browser/menu/src/test/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButtonTest.kt

@@ -6,18 +6,26 @@ package mozilla.components.browser.menu.item
 
 import android.view.LayoutInflater
 import android.view.View
+import android.view.ViewTreeObserver
 import android.widget.CheckBox
+import androidx.appcompat.widget.SwitchCompat
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.browser.menu.BrowserMenu
 import mozilla.components.browser.menu.R
 import mozilla.components.concept.menu.candidate.CompoundMenuCandidate
+import mozilla.components.support.test.any
+import mozilla.components.support.test.argumentCaptor
+import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers
+import org.mockito.Mockito.doReturn
 import org.mockito.Mockito.mock
+import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.verify
 
@@ -114,6 +122,64 @@ class BrowserMenuCompoundButtonTest {
         )
     }
 
+    @Test
+    fun `GIVEN the View is attached to Window WHEN bind is called THEN the layout direction is not updated`() {
+        val item = SimpleTestBrowserCompoundButton("Hello") {}
+        val menu = mock(BrowserMenu::class.java)
+        val view: SwitchCompat = mock()
+        doReturn(true).`when`(view).isAttachedToWindow
+        doReturn(mock<ViewTreeObserver>()).`when`(view).viewTreeObserver
+
+        item.bind(menu, view)
+
+        verify(view, never()).layoutDirection = ArgumentMatchers.anyInt()
+    }
+
+    @Test
+    fun `GIVEN the View is not attached to Window WHEN bind is called THEN the layout direction is changed to locale`() {
+        val item = SimpleTestBrowserCompoundButton("Hello") {}
+        val menu = mock(BrowserMenu::class.java)
+        val view: SwitchCompat = mock()
+        doReturn(false).`when`(view).isAttachedToWindow
+        doReturn(mock<ViewTreeObserver>()).`when`(view).viewTreeObserver
+
+        item.bind(menu, view)
+
+        verify(view).layoutDirection = View.LAYOUT_DIRECTION_LOCALE
+    }
+
+    @Test
+    fun `GIVEN the View is not attached to Window WHEN bind is called THEN the a viewTreeObserver for preDraw is set`() {
+        val item = SimpleTestBrowserCompoundButton("Hello") {}
+        val menu = mock(BrowserMenu::class.java)
+        val view: SwitchCompat = mock()
+        val viewTreeObserver: ViewTreeObserver = mock()
+        doReturn(false).`when`(view).isAttachedToWindow
+        doReturn(viewTreeObserver).`when`(view).viewTreeObserver
+
+        item.bind(menu, view)
+
+        verify(viewTreeObserver).addOnPreDrawListener(any())
+    }
+
+    @Test
+    fun `GIVEN a view with updated layout direction WHEN it is about to be drawn THEN the layout direction reset`() {
+        val item = SimpleTestBrowserCompoundButton("Hello") {}
+        val menu = mock(BrowserMenu::class.java)
+        val view: SwitchCompat = mock()
+        val viewTreeObserver: ViewTreeObserver = mock()
+        doReturn(false).`when`(view).isAttachedToWindow
+        doReturn(viewTreeObserver).`when`(view).viewTreeObserver
+        val captor = argumentCaptor<ViewTreeObserver.OnPreDrawListener>()
+
+        item.bind(menu, view)
+        verify(viewTreeObserver).addOnPreDrawListener(captor.capture())
+
+        captor.value.onPreDraw()
+        verify(viewTreeObserver).removeOnPreDrawListener(captor.value)
+        verify(view).layoutDirection = View.LAYOUT_DIRECTION_INHERIT
+    }
+
     class SimpleTestBrowserCompoundButton(
         label: String,
         initialState: () -> Boolean = { false },

docs/changelog.md

@@ -6,19 +6,20 @@ permalink: /changelog/
 
 # 75.0.0-SNAPSHOT (In Development)
 
-* [Commits](https://github.com/mozilla-mobile/android-components/compare/v74.0.0...master)
-* [Milestone](https://github.com/mozilla-mobile/android-components/milestone/136?closed=1)
+* [Commits](https://github.com/mozilla-mobile/android-components/compare/v75.0.0...master)
+* [Milestone](https://github.com/mozilla-mobile/android-components/milestone/137?closed=1)
 * [Dependencies](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Dependencies.kt)
 * [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 #10133](https://github.com/mozilla-mobile/android-components/issues/10133) - A BrowserMenuCompoundButton used in our BrowserMenu setup with a DynamicWidthRecyclerView is not clipped anymore.
   * 🌟️ New StickyHeaderLinearLayoutManager and StickyFooterLinearLayoutManager that can be used to keep an item from being scrolled off-screen.
   * To use this set `isSticky = true` for any menu item of the menu. Since only one sticky item is supported if more items have this property the sticky item will be the one closest to the top the menu anchor.
-  * 🚒 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.
-  * 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/10005) - Fix a recent issue with BrowserMenu#show() - endOfMenuAlwaysVisible not being applied.
-  * 🚒 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.
+  * 🚒 Bug fixed [issue #10032](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.
+  * 🚒 Bug fixed [issue #10005](https://github.com/mozilla-mobile/android-components/issues/10005) - Fix a recent issue with BrowserMenu#show() - endOfMenuAlwaysVisible not being applied.
+  * 🚒 Bug fixed [issue #9922](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.
   * 🌟️ BrowserMenu support a bottom collapsed/expandable layout through a new ExpandableLayout that will wrap a menu layout before being used in a PopupWindow and automatically allow the collapse/expand behaviors.
   * To use this set `isCollapsingMenuLimit = true` for any menu item of a bottom anchored menu.
   * 🌟️ `WebExtensionBrowserMenuBuilder` provide a new way to customize how items look like via `Style()` where the `tintColor`, `backPressDrawable` and `addonsManagerDrawable` can be customized.