Commit 54b465ef authored by Mugurell's avatar Mugurell Committed by mergify[bot]
Browse files

For #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.
parent 201765eb
......@@ -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()
......
......@@ -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 },
......
......@@ -14,6 +14,7 @@ permalink: /changelog/
* **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.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment