From 119362e1b20033e5b52eb42168f3a6ad54b1aecc Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 22 Apr 2021 12:14:36 +0300 Subject: [PATCH] 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. --- .../menu/item/BrowserMenuCompoundButton.kt | 18 +++++ .../item/BrowserMenuCompoundButtonTest.kt | 66 +++++++++++++++++++ docs/changelog.md | 11 ++-- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButton.kt b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButton.kt index 86b6550308a..d8c3d291e5e 100644 --- a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButton.kt +++ b/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() diff --git a/components/browser/menu/src/test/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButtonTest.kt b/components/browser/menu/src/test/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButtonTest.kt index d77fb418784..255e0a244f8 100644 --- a/components/browser/menu/src/test/java/mozilla/components/browser/menu/item/BrowserMenuCompoundButtonTest.kt +++ b/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()).`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()).`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() + + 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 }, diff --git a/docs/changelog.md b/docs/changelog.md index b5d16827585..9effe09e9bd 100644 --- a/docs/changelog.md +++ b/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.