From ebfc9edc79545dfa8f647cef6c5d24303e4f5fa8 Mon Sep 17 00:00:00 2001 From: mike a Date: Mon, 4 Mar 2024 14:29:32 -0800 Subject: [PATCH] Bug 1879648 - Disable menu toolbar when nav bar is enabled --- .../mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt | 5 +++-- .../org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt | 6 ++++-- .../fenix/components/toolbar/BrowserToolbarViewTest.kt | 2 ++ .../mozilla/fenix/customtabs/CustomTabToolbarMenuTest.kt | 3 +++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index 01f88e34a..62159623e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -72,6 +72,7 @@ open class DefaultToolbarMenu( private val shouldDeleteDataOnQuit = context.settings().shouldDeleteBrowsingDataOnQuit private val shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar + private val shouldShowMenuToolbar = !IncompleteRedesignToolbarFeature(context.settings()).isEnabled private val shouldShowTopSites = context.settings().showTopSitesFeature private val accountManager = FenixAccountManager(context) @@ -411,7 +412,7 @@ open class DefaultToolbarMenu( val coreMenuItems by lazy { val menuItems = listOfNotNull( - if (shouldUseBottomToolbar) null else menuToolbar, + if (shouldUseBottomToolbar || !shouldShowMenuToolbar) null else menuToolbar, newTabItem, BrowserMenuDivider(), bookmarksItem, @@ -437,7 +438,7 @@ open class DefaultToolbarMenu( settingsItem, if (shouldDeleteDataOnQuit) deleteDataOnQuit else null, if (shouldUseBottomToolbar) BrowserMenuDivider() else null, - if (shouldUseBottomToolbar) menuToolbar else null, + if (shouldUseBottomToolbar && shouldShowMenuToolbar) menuToolbar else null, ) menuItems diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt index 9a0834b13..2de931987 100644 --- a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenu.kt @@ -22,6 +22,7 @@ import mozilla.components.browser.state.selector.findCustomTab import mozilla.components.browser.state.state.CustomTabSessionState import mozilla.components.browser.state.store.BrowserStore import org.mozilla.fenix.R +import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature import org.mozilla.fenix.components.toolbar.ToolbarMenu import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getStringWithArgSafe @@ -54,6 +55,7 @@ class CustomTabToolbarMenu( @VisibleForTesting internal val session: CustomTabSessionState? get() = sessionId?.let { store.state.findCustomTab(it) } private val appName = context.getString(R.string.app_name) + private val shouldShowMenuToolbar = !IncompleteRedesignToolbarFeature(context.settings()).isEnabled override val menuToolbar by lazy { val back = BrowserMenuItemToolbar.TwoStateButton( @@ -119,7 +121,7 @@ class CustomTabToolbarMenu( } ?: false private val menuItems by lazy { - val menuItems = listOf( + val menuItems = listOfNotNull( poweredBy.apply { visible = { !isSandboxCustomTab } }, BrowserMenuDivider().apply { visible = { !isSandboxCustomTab } }, desktopMode, @@ -127,7 +129,7 @@ class CustomTabToolbarMenu( openInApp.apply { visible = ::shouldShowOpenInApp }, openInFenix.apply { visible = { !isSandboxCustomTab } }, BrowserMenuDivider(), - menuToolbar, + if (shouldShowMenuToolbar) menuToolbar else null, ) if (shouldReverseItems) { menuItems.reversed() diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarViewTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarViewTest.kt index 2fad1a270..422ede477 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarViewTest.kt @@ -20,6 +20,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings import mozilla.components.ui.widgets.behavior.ViewPosition as MozacToolbarPosition @@ -41,6 +42,7 @@ class BrowserToolbarViewTest { every { testContext.components.useCases } returns mockk(relaxed = true) every { testContext.components.core } returns mockk(relaxed = true) every { testContext.components.publicSuffixList } returns PublicSuffixList(testContext) + every { testContext.settings() } returns settings toolbarView = BrowserToolbarView( context = testContext, diff --git a/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenuTest.kt b/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenuTest.kt index 7caab9caf..7f2279e7f 100644 --- a/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenuTest.kt +++ b/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabToolbarMenuTest.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.customtabs import android.content.Context +import io.mockk.every import io.mockk.mockk import io.mockk.spyk import mozilla.components.browser.state.state.BrowserState @@ -15,6 +16,7 @@ import mozilla.components.browser.state.store.BrowserStore import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.mozilla.fenix.ext.settings class CustomTabToolbarMenuTest { @@ -26,6 +28,7 @@ class CustomTabToolbarMenuTest { @Before fun setUp() { context = mockk(relaxed = true) + every { context.settings() } returns mockk(relaxed = true) firefoxCustomTab = createCustomTab(url = "https://firefox.com", id = "123")