commit 7331a2f87d1916c5cea5e9f4437a48fade7ae17a parent 5cd6bc910b97cb6b0cadbab1fd75174e9fee3dbc Author: rmalicdem <rmalicdem@mozilla.com> Date: Thu, 20 Nov 2025 02:47:19 +0000 Bug 1997127 - Hide tab strip buttons and show end-browser buttons when composable toolbar enabled r=android-reviewers,Roger Differential Revision: https://phabricator.services.mozilla.com/D273086 Diffstat:
7 files changed, 60 insertions(+), 32 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -206,9 +206,7 @@ import org.mozilla.fenix.ext.getBottomToolbarHeight import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.getTopToolbarHeight import org.mozilla.fenix.ext.hideToolbar -import org.mozilla.fenix.ext.isTallWindow import org.mozilla.fenix.ext.isToolbarAtBottom -import org.mozilla.fenix.ext.isWideWindow import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.pixelSizeFor @@ -1440,9 +1438,9 @@ abstract class BaseBrowserFragment : ): @Composable () -> Unit = { FirefoxTheme { TabStrip( - // Show action buttons only if the navigation bar (which has the same buttons) is not showing. + // Show action buttons only if composable toolbar is not enabled. showActionButtons = - context?.settings()?.shouldUseExpandedToolbar == false || !isTallWindow() || isWideWindow(), + context?.settings()?.shouldUseComposableToolbar == false, onAddTabClick = { if (activity.settings().enableHomepageAsNewTab) { requireComponents.useCases.fenixBrowserUseCases.addNewHomepageTab( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt @@ -487,10 +487,11 @@ class TabPreview @JvmOverloads constructor( val settings = context.settings() val isWideScreen = context.isWideWindow() val tabStripEnabled = settings.isTabStripEnabled + val shareShortcutEnabled = ShortcutType.fromValue(settings.toolbarSimpleShortcutKey) == ShortcutType.SHARE return listOf( ToolbarActionConfig(ToolbarAction.Share) { - isWideScreen && !tabStripEnabled + isWideScreen && !tabStripEnabled && !shareShortcutEnabled }, ).filter { config -> config.isVisible() @@ -525,15 +526,16 @@ class TabPreview @JvmOverloads constructor( ?.toToolbarAction(tab).takeIf { useCustomPrimary } ?: ToolbarAction.NewTab return listOf( + ToolbarActionConfig(ToolbarAction.Share) { + tabStripEnabled && isWideWindow && (!shouldUseExpandedToolbar || !isTallWindow) && + primarySlotAction == ToolbarAction.Share + }, ToolbarActionConfig(primarySlotAction) { - !tabStripEnabled && (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) && + (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) && tab?.content?.url != ABOUT_HOME_URL }, ToolbarActionConfig(ToolbarAction.TabCounter) { - !tabStripEnabled && (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) - }, - ToolbarActionConfig(ToolbarAction.Share) { - tabStripEnabled && isWideWindow && (!shouldUseExpandedToolbar || !isTallWindow) + !shouldUseExpandedToolbar || !isTallWindow || isWideWindow }, ToolbarActionConfig(ToolbarAction.Menu) { !shouldUseExpandedToolbar || !isTallWindow || isWideWindow diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt @@ -749,7 +749,7 @@ class BrowserToolbarMiddleware( ToolbarActionConfig(ToolbarAction.Translate) { browserScreenStore.state.pageTranslationStatus.isTranslationPossible && isWideScreen && FxNimbus.features.translations.value().mainFlowToolbarEnabled && - !(translateShortcutEnabled && !tabStripEnabled) + !translateShortcutEnabled }, ToolbarActionConfig(ToolbarAction.Share) { isWideScreen && !tabStripEnabled && !shareShortcutEnabled @@ -771,14 +771,15 @@ class BrowserToolbarMiddleware( ?.toToolbarAction().takeIf { useCustomPrimary } ?: ToolbarAction.NewTab val configs = listOf( + ToolbarActionConfig(ToolbarAction.Share) { + tabStripEnabled && isWideWindow && (!shouldUseExpandedToolbar || !isTallWindow) && + primarySlotAction != ToolbarAction.Share + }, ToolbarActionConfig(primarySlotAction) { - !tabStripEnabled && (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) + !shouldUseExpandedToolbar || !isTallWindow || isWideWindow }, ToolbarActionConfig(ToolbarAction.TabCounter) { - !tabStripEnabled && (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) - }, - ToolbarActionConfig(ToolbarAction.Share) { - tabStripEnabled && isWideWindow && (!shouldUseExpandedToolbar || !isTallWindow) + !shouldUseExpandedToolbar || !isTallWindow || isWideWindow }, ToolbarActionConfig(ToolbarAction.Menu) { !shouldUseExpandedToolbar || !isTallWindow || isWideWindow diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -100,9 +100,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getBottomToolbarHeight import org.mozilla.fenix.ext.getTopToolbarHeight import org.mozilla.fenix.ext.hideToolbar -import org.mozilla.fenix.ext.isTallWindow import org.mozilla.fenix.ext.isToolbarAtBottom -import org.mozilla.fenix.ext.isWideWindow import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.recordEventInNimbus import org.mozilla.fenix.ext.requireComponents @@ -1072,9 +1070,9 @@ class HomeFragment : Fragment() { FirefoxTheme { TabStrip( isSelectDisabled = isSelectDisabled, - // Show action buttons only if the navigation bar (which has the same buttons) is not showing. + // Show action buttons only if composable toolbar is not enabled. showActionButtons = - context?.settings()?.shouldUseExpandedToolbar == false || !isTallWindow() || isWideWindow(), + context?.settings()?.shouldUseComposableToolbar == false, onAddTabClick = { if (requireContext().settings().enableHomepageAsNewTab) { requireComponents.useCases.fenixBrowserUseCases.addNewHomepageTab( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddleware.kt @@ -326,12 +326,11 @@ class BrowserToolbarMiddleware( private fun buildEndBrowserActions(): List<Action> { val isWideWindow = environment?.fragment?.isWideWindow() == true val isTallWindow = environment?.fragment?.isTallWindow() == true - val tabStripEnabled = environment?.context?.settings()?.isTabStripEnabled == true val shouldUseExpandedToolbar = environment?.context?.settings()?.shouldUseExpandedToolbar == true return listOf( HomeToolbarActionConfig(HomeToolbarAction.TabCounter) { - !tabStripEnabled && (!shouldUseExpandedToolbar || !isTallWindow || isWideWindow) + !shouldUseExpandedToolbar || !isTallWindow || isWideWindow }, HomeToolbarActionConfig(HomeToolbarAction.Menu) { !shouldUseExpandedToolbar || !isTallWindow || isWideWindow diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt @@ -1566,13 +1566,17 @@ class BrowserToolbarMiddlewareTest { navController = navController, ) - assertEquals(1, toolbarStore.state.displayState.browserActionsEnd.size) - val toolbarButton = toolbarStore.state.displayState.browserActionsEnd[0] - assertNotEquals(expectedShareButton(), toolbarButton) + assertEquals(3, toolbarStore.state.displayState.browserActionsEnd.size) + val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes + val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction + val menuButton = toolbarStore.state.displayState.browserActionsEnd[2] as ActionButtonRes + assertEquals(expectedNewTabButton(), newTabButton) + assertEqualsTabCounterButton(expectedTabCounterButton(), tabCounterButton) + assertEquals(expectedMenuButton(), menuButton) } @Test - fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show menu`() = runTest { + fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show new tab, tab counter and menu`() = runTest { every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns true configuration = Configuration().apply { @@ -1604,8 +1608,12 @@ class BrowserToolbarMiddlewareTest { navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(0, navigationActions.size) toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd - assertEquals(1, toolbarBrowserActions.size) - val menuButton = toolbarBrowserActions[0] as ActionButtonRes + assertEquals(3, toolbarBrowserActions.size) + val newTabButton = toolbarBrowserActions[0] as ActionButtonRes + val tabCounterButton = toolbarBrowserActions[1] as TabCounterAction + val menuButton = toolbarBrowserActions[2] as ActionButtonRes + assertEquals(expectedNewTabButton(), newTabButton) + assertEqualsTabCounterButton(expectedTabCounterButton(), tabCounterButton) assertEquals(expectedMenuButton(), menuButton) } @@ -2922,7 +2930,6 @@ class BrowserToolbarMiddlewareTest { screenWidthDp = 700 } every { mockContext.resources.configuration } returns configuration - every { settings.isTabStripEnabled } returns false every { settings.toolbarSimpleShortcutKey } returns ShortcutType.TRANSLATE.value val browserScreenStore = buildBrowserScreenStore() val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) @@ -3124,6 +3131,27 @@ class BrowserToolbarMiddlewareTest { } @Test + fun `GIVEN simple toolbar use share shortcut AND wide window with tabstrip enabled WHEN initializing toolbar THEN only show one Share in end browser actions`() { + configuration = Configuration().apply { + screenHeightDp = 400 + screenWidthDp = 700 + } + every { settings.shouldShowToolbarCustomization } returns true + every { settings.isTabStripEnabled } returns true + every { settings.toolbarSimpleShortcutKey } returns ShortcutType.SHARE.value + + val toolbarStore = buildStore() + + assertEquals(3, toolbarStore.state.displayState.browserActionsEnd.size) + val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes + val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction + val menuButton = toolbarStore.state.displayState.browserActionsEnd[2] as ActionButtonRes + assertEquals(expectedShareButton(), shareButton) + assertEqualsTabCounterButton(expectedTabCounterButton(), tabCounterButton) + assertNotEquals(expectedShareButton(), menuButton) + } + + @Test fun `GIVEN expanded toolbar use translate shortcut AND current page is not translated WHEN initializing toolbar THEN show Translate in navigation actions`() = runTest { every { settings.shouldShowToolbarCustomization } returns true every { settings.shouldUseExpandedToolbar } returns true diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt @@ -382,7 +382,7 @@ class BrowserToolbarMiddlewareTest { } @Test - fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show menu`() = runTest { + fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show tab counter and menu`() = runTest { every { testContext.settings().shouldUseExpandedToolbar } returns true every { testContext.settings().isTabStripEnabled } returns true configuration = Configuration().apply { @@ -409,8 +409,10 @@ class BrowserToolbarMiddlewareTest { navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(0, navigationActions.size) toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd - assertEquals(1, toolbarBrowserActions.size) - val menuButton = toolbarBrowserActions[0] as ActionButtonRes + assertEquals(2, toolbarBrowserActions.size) + val tabCounterButton = toolbarBrowserActions[0] as TabCounterAction + val menuButton = toolbarBrowserActions[1] as ActionButtonRes + assertEqualsToolbarButton(expectedToolbarButton(), tabCounterButton) assertEquals(expectedMenuButton(), menuButton) }