From fe5bb39b3a9ef79e9148c91cb43aef35bcec2594 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Thu, 15 Apr 2021 16:08:39 -0400 Subject: [PATCH] Closes #10091 Added style property to WebExtensionBrowserMenuBuilder's constructor --- .../menu/WebExtensionBrowserMenuBuilder.kt | 34 ++++++++++----- .../menu/item/BrowserMenuHighlightableItem.kt | 2 +- .../browser/menu/item/BrowserMenuImageText.kt | 7 +++- .../BrowserMenuImageTextCheckboxButton.kt | 2 +- .../WebExtensionBrowserMenuBuilderTest.kt | 41 +++++++++++++++++++ docs/changelog.md | 10 +---- .../samples/browser/DefaultComponents.kt | 4 +- 7 files changed, 76 insertions(+), 24 deletions(-) diff --git a/components/browser/menu/src/main/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilder.kt b/components/browser/menu/src/main/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilder.kt index 434a6069aad..3947541aca9 100644 --- a/components/browser/menu/src/main/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilder.kt +++ b/components/browser/menu/src/main/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilder.kt @@ -6,6 +6,7 @@ package mozilla.components.browser.menu import android.content.Context import androidx.annotation.ColorRes +import androidx.annotation.DrawableRes import mozilla.components.browser.menu.item.BackPressMenuItem import mozilla.components.browser.menu.item.BrowserMenuDivider import mozilla.components.browser.menu.item.BrowserMenuImageText @@ -21,8 +22,7 @@ import mozilla.components.browser.state.store.BrowserStore * the web extension menu item would return an add-on manager menu item instead. * * @param store [BrowserStore] required to render web extension browser actions - * @param webExtIconTintColorResource Optional ID of color resource to tint the icons of back and - * add-ons manager menu items. + * @param style Indicates how items should look like. * @param onAddonsManagerTapped Callback to be invoked when add-ons manager menu item is selected. * @param appendExtensionSubMenuAtStart Used when the menu does not have a [WebExtensionPlaceholderMenuItem] * to specify the place the extensions sub-menu should be inserted. True if web extension sub menu @@ -35,7 +35,7 @@ class WebExtensionBrowserMenuBuilder( extras: Map = emptyMap(), endOfMenuAlwaysVisible: Boolean = false, private val store: BrowserStore, - @ColorRes private val webExtIconTintColorResource: Int = NO_ID, + private val style: Style = Style(), private val onAddonsManagerTapped: () -> Unit = {}, private val appendExtensionSubMenuAtStart: Boolean = false ) : BrowserMenuBuilder(items, extras, endOfMenuAlwaysVisible) { @@ -64,14 +64,14 @@ class WebExtensionBrowserMenuBuilder( val webExtMenuItem = if (filteredExtensionMenuItems.isNotEmpty()) { val backPressMenuItem = BackPressMenuItem( label = context.getString(R.string.mozac_browser_menu_addons), - imageResource = R.drawable.mozac_ic_back, - iconTintColorResource = webExtIconTintColorResource + imageResource = style.backPressMenuItemDrawableRes, + iconTintColorResource = style.webExtIconTintColorResource ) val addonsManagerMenuItem = BrowserMenuImageText( label = context.getString(R.string.mozac_browser_menu_addons_manager), - imageResource = R.drawable.mozac_ic_extensions, - iconTintColorResource = webExtIconTintColorResource + imageResource = style.addonsManagerMenuItemDrawableRes, + iconTintColorResource = style.webExtIconTintColorResource ) { onAddonsManagerTapped.invoke() } @@ -91,16 +91,16 @@ class WebExtensionBrowserMenuBuilder( ParentBrowserMenuItem( label = context.getString(R.string.mozac_browser_menu_addons), - imageResource = R.drawable.mozac_ic_extensions, - iconTintColorResource = webExtIconTintColorResource, + imageResource = style.addonsManagerMenuItemDrawableRes, + iconTintColorResource = style.webExtIconTintColorResource, subMenu = webExtMenu, endOfMenuAlwaysVisible = endOfMenuAlwaysVisible ) } else { BrowserMenuImageText( label = context.getString(R.string.mozac_browser_menu_addons), - imageResource = R.drawable.mozac_ic_extensions, - iconTintColorResource = webExtIconTintColorResource + imageResource = style.addonsManagerMenuItemDrawableRes, + iconTintColorResource = style.webExtIconTintColorResource ) { onAddonsManagerTapped.invoke() } @@ -126,4 +126,16 @@ class WebExtensionBrowserMenuBuilder( val adapter = BrowserMenuAdapter(context, menuItems) return BrowserMenu(adapter) } + + /** + * Allows to customize how items should look like. + */ + data class Style( + @ColorRes + val webExtIconTintColorResource: Int = NO_ID, + @DrawableRes + val backPressMenuItemDrawableRes: Int = R.drawable.mozac_ic_back, + @DrawableRes + val addonsManagerMenuItemDrawableRes: Int = R.drawable.mozac_ic_extensions + ) } diff --git a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuHighlightableItem.kt b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuHighlightableItem.kt index e6db9c6b089..e98dfa6bfe5 100644 --- a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuHighlightableItem.kt +++ b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuHighlightableItem.kt @@ -42,7 +42,7 @@ private val defaultHighlight = BrowserMenuHighlightableItem.Highlight(0, 0, 0, 0 class BrowserMenuHighlightableItem( private val label: String, @DrawableRes private val startImageResource: Int, - @ColorRes private val iconTintColorResource: Int = NO_ID, + @ColorRes iconTintColorResource: Int = NO_ID, @ColorRes private val textColorResource: Int = NO_ID, override val isCollapsingMenuLimit: Boolean = false, override val isSticky: Boolean = false, diff --git a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageText.kt b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageText.kt index b1e05b0d024..a6e1dd95f8e 100644 --- a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageText.kt +++ b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageText.kt @@ -10,6 +10,7 @@ import android.widget.ImageView import android.widget.TextView import androidx.annotation.ColorRes import androidx.annotation.DrawableRes +import androidx.annotation.VisibleForTesting import androidx.appcompat.widget.AppCompatImageView import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat.getColor @@ -52,9 +53,11 @@ internal fun TextView.setColorResource(@ColorRes textColorResource: Int) { open class BrowserMenuImageText( private val label: String, @DrawableRes - private val imageResource: Int, + @VisibleForTesting + internal val imageResource: Int, @ColorRes - private val iconTintColorResource: Int = NO_ID, + @VisibleForTesting + internal val iconTintColorResource: Int = NO_ID, @ColorRes private val textColorResource: Int = NO_ID, override val isCollapsingMenuLimit: Boolean = false, diff --git a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageTextCheckboxButton.kt b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageTextCheckboxButton.kt index bce0324e89b..669678bf6ef 100644 --- a/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageTextCheckboxButton.kt +++ b/components/browser/menu/src/main/java/mozilla/components/browser/menu/item/BrowserMenuImageTextCheckboxButton.kt @@ -37,7 +37,7 @@ import mozilla.components.support.ktx.android.util.dpToPx class BrowserMenuImageTextCheckboxButton( @DrawableRes imageResource: Int, private val label: String, - @ColorRes internal val iconTintColorResource: Int = NO_ID, + @ColorRes iconTintColorResource: Int = NO_ID, @ColorRes internal val textColorResource: Int = NO_ID, @VisibleForTesting internal val labelListener: () -> Unit, @DrawableRes val primaryStateIconResource: Int, diff --git a/components/browser/menu/src/test/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilderTest.kt b/components/browser/menu/src/test/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilderTest.kt index 6c8c7cfa638..104fc6f8a0d 100644 --- a/components/browser/menu/src/test/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilderTest.kt +++ b/components/browser/menu/src/test/java/mozilla/components/browser/menu/WebExtensionBrowserMenuBuilderTest.kt @@ -55,6 +55,47 @@ class WebExtensionBrowserMenuBuilderTest { assertTrue(isAddonsManagerTapped) } + @Test + fun `GIVEN style is provided WHEN creating extension menu THEN styles should be applied to items`() { + val browserAction = WebExtensionBrowserAction("browser_action", true, mock(), "", 0, 0) {} + val extensions = mapOf( + "id" to WebExtensionState( + "id", + "url", + "name", + true, + browserAction = browserAction + ) + ) + + val store = BrowserStore(BrowserState(extensions = extensions)) + val style = WebExtensionBrowserMenuBuilder.Style( + addonsManagerMenuItemDrawableRes = R.drawable.mozac_ic_extensions, + backPressMenuItemDrawableRes = R.drawable.mozac_ic_back + ) + val builder = WebExtensionBrowserMenuBuilder( + listOf(mockMenuItem()), + store = store, + style = style, + appendExtensionSubMenuAtStart = true + ) + + val menu = builder.build(testContext) + val anchor = ImageButton(testContext) + menu.show(anchor) + + val parentMenuItem = menu.adapter.visibleItems[0] as ParentBrowserMenuItem + val subMenuItemIndex = parentMenuItem.subMenu.adapter.visibleItems.lastIndex + val backPressMenuItem = parentMenuItem.subMenu.adapter.visibleItems[0] as BackPressMenuItem + val addonsManagerItem = parentMenuItem.subMenu.adapter.visibleItems[subMenuItemIndex] as BrowserMenuImageText + + assertEquals(style.backPressMenuItemDrawableRes, backPressMenuItem.imageResource) + assertEquals(style.webExtIconTintColorResource, backPressMenuItem.iconTintColorResource) + + assertEquals(style.webExtIconTintColorResource, addonsManagerItem.iconTintColorResource) + assertEquals(style.webExtIconTintColorResource, addonsManagerItem.iconTintColorResource) + } + @Test fun `web extension sub menu add-ons manager sub menu item invokes onAddonsManagerTapped when clicked`() { val browserAction = WebExtensionBrowserAction("browser_action", true, mock(), "", 0, 0) {} diff --git a/docs/changelog.md b/docs/changelog.md index c40287ac87c..b5d16827585 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -16,19 +16,13 @@ permalink: /changelog/ * **browser-menu**: * 🌟️ 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. - -* **browser-menu**: * 🚒 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. - -* **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. - -* **browser-menu** * 🌟️ 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. + * ⚠️ **This is a breaking change**: `WebExtensionBrowserMenuBuilder.webExtIconTintColorResource` constructor parameter has been removed, please use `WebExtensionBrowserMenuBuilder`.`Style` instead. For more details see [issue #9787](https://github.com/mozilla-mobile/android-components/issues/10091). * **browser-toolbar** * **feature-session** diff --git a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt index bba0ca30bf8..92f77f4d27b 100644 --- a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt +++ b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt @@ -273,7 +273,9 @@ open class DefaultComponents(private val applicationContext: Context) { WebExtensionBrowserMenuBuilder( menuItems, store = store, - webExtIconTintColorResource = R.color.photonGrey90, + style = WebExtensionBrowserMenuBuilder.Style( + webExtIconTintColorResource = R.color.photonGrey90 + ), onAddonsManagerTapped = { val intent = Intent(applicationContext, AddonsActivity::class.java) intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK