From d2ddcb01ae3596fdd081b9dc87bf8b984f54cd79 Mon Sep 17 00:00:00 2001 From: "alexandra.virvara" Date: Fri, 28 Jul 2023 14:36:48 +0300 Subject: [PATCH] Bug 1833693: refactor menu items list creation out of TabsTray Banner --- .../mozilla/fenix/tabstray/TabsTrayBanner.kt | 156 +++---------- .../fenix/tabstray/ext/TabsTrayState.kt | 172 +++++++++++++++ .../fenix/tabstray/TabsTrayStateTest.kt | 206 ++++++++++++++++++ 3 files changed, 405 insertions(+), 129 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStateTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayBanner.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayBanner.kt index 00fe9481fc..23bc81ecf4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayBanner.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayBanner.kt @@ -34,6 +34,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.dimensionResource @@ -56,6 +57,7 @@ import org.mozilla.fenix.compose.ContextualMenu import org.mozilla.fenix.compose.Divider import org.mozilla.fenix.compose.MenuItem import org.mozilla.fenix.compose.annotation.LightDarkPreview +import org.mozilla.fenix.tabstray.ext.getMenuItems import org.mozilla.fenix.theme.FirefoxTheme import kotlin.math.max @@ -123,30 +125,39 @@ fun TabsTrayBanner( }.value ?: false var hasAcknowledgedBanner by remember { mutableStateOf(false) } + val menuItems = multiselectMode.getMenuItems( + resources = LocalContext.current.resources, + shouldShowInactiveButton = isInDebugMode, + onBookmarkSelectedTabsClick = onBookmarkSelectedTabsClick, + onCloseSelectedTabsClick = onDeleteSelectedTabsClick, + onMakeSelectedTabsInactive = onForceSelectedTabsAsInactiveClick, + + selectedPage = selectedPage, + normalTabCount = normalTabCount, + privateTabCount = privateTabCount, + onTabSettingsClick = onTabSettingsClick, + onRecentlyClosedClick = onRecentlyClosedClick, + onEnterMultiselectModeClick = { tabsTrayStore.dispatch(TabsTrayAction.EnterSelectMode) }, + onShareAllTabsClick = onShareAllTabsClick, + onDeleteAllTabsClick = onDeleteAllTabsClick, + onAccountSettingsClick = onAccountSettingsClick, + ) + Column { if (multiselectMode is TabsTrayState.Mode.Select) { MultiSelectBanner( + menuItems = menuItems, selectedTabCount = multiselectMode.selectedTabs.size, - shouldShowInactiveButton = isInDebugMode, onExitSelectModeClick = { tabsTrayStore.dispatch(TabsTrayAction.ExitSelectMode) }, onSaveToCollectionsClick = onSaveToCollectionClick, onShareSelectedTabs = onShareSelectedTabsClick, - onBookmarkSelectedTabsClick = onBookmarkSelectedTabsClick, - onCloseSelectedTabsClick = onDeleteSelectedTabsClick, - onMakeSelectedTabsInactive = onForceSelectedTabsAsInactiveClick, ) } else { SingleSelectBanner( - onTabPageIndicatorClicked = onTabPageIndicatorClicked, + menuItems = menuItems, selectedPage = selectedPage, normalTabCount = normalTabCount, - privateTabCount = privateTabCount, - onEnterMultiselectModeClick = { tabsTrayStore.dispatch(TabsTrayAction.EnterSelectMode) }, - onShareAllTabsClick = onShareAllTabsClick, - onTabSettingsClick = onTabSettingsClick, - onRecentlyClosedClick = onRecentlyClosedClick, - onAccountSettingsClick = onAccountSettingsClick, - onDeleteAllTabsClick = onDeleteAllTabsClick, + onTabPageIndicatorClicked = onTabPageIndicatorClicked, onDismissClick = onDismissClick, ) } @@ -176,16 +187,10 @@ fun TabsTrayBanner( @Suppress("LongMethod", "LongParameterList") @Composable private fun SingleSelectBanner( + menuItems: List, selectedPage: Page, normalTabCount: Int, - privateTabCount: Int, onTabPageIndicatorClicked: (Page) -> Unit, - onEnterMultiselectModeClick: () -> Unit, - onShareAllTabsClick: () -> Unit, - onTabSettingsClick: () -> Unit, - onRecentlyClosedClick: () -> Unit, - onAccountSettingsClick: () -> Unit, - onDeleteAllTabsClick: () -> Unit, onDismissClick: () -> Unit, ) { val selectedColor = FirefoxTheme.colors.iconActive @@ -273,17 +278,7 @@ private fun SingleSelectBanner( .testTag(TabsTrayTestTag.threeDotButton), ) { ContextualMenu( - menuItems = generateSingleSelectBannerMenuItems( - selectedPage, - normalTabCount, - privateTabCount, - onTabSettingsClick, - onRecentlyClosedClick, - onEnterMultiselectModeClick, - onShareAllTabsClick, - onDeleteAllTabsClick, - onAccountSettingsClick, - ), + menuItems = menuItems, showMenu = showMenu, offset = DpOffset(x = 0.dp, y = -ICON_SIZE), onDismissRequest = { showMenu = false }, @@ -298,79 +293,6 @@ private fun SingleSelectBanner( } } -@Suppress("LongParameterList") -@Composable -private fun generateSingleSelectBannerMenuItems( - selectedPage: Page, - normalTabCount: Int, - privateTabCount: Int, - onTabSettingsClick: () -> Unit, - onRecentlyClosedClick: () -> Unit, - onEnterMultiselectModeClick: () -> Unit, - onShareAllTabsClick: () -> Unit, - onDeleteAllTabsClick: () -> Unit, - onAccountSettingsClick: () -> Unit, -): List { - val tabSettingsItem = MenuItem( - title = stringResource(id = R.string.tab_tray_menu_tab_settings), - testTag = TabsTrayTestTag.tabSettings, - onClick = onTabSettingsClick, - ) - val recentlyClosedTabsItem = MenuItem( - title = stringResource(id = R.string.tab_tray_menu_recently_closed), - testTag = TabsTrayTestTag.recentlyClosedTabs, - onClick = onRecentlyClosedClick, - ) - val enterSelectModeItem = MenuItem( - title = stringResource(id = R.string.tabs_tray_select_tabs), - testTag = TabsTrayTestTag.selectTabs, - onClick = onEnterMultiselectModeClick, - ) - val shareAllTabsItem = MenuItem( - title = stringResource(id = R.string.tab_tray_menu_item_share), - testTag = TabsTrayTestTag.shareAllTabs, - onClick = onShareAllTabsClick, - ) - val deleteAllTabsItem = MenuItem( - title = stringResource(id = R.string.tab_tray_menu_item_close), - testTag = TabsTrayTestTag.closeAllTabs, - onClick = onDeleteAllTabsClick, - ) - val accountSettingsItem = MenuItem( - title = stringResource(id = R.string.tab_tray_menu_account_settings), - testTag = TabsTrayTestTag.accountSettings, - onClick = onAccountSettingsClick, - ) - return when { - selectedPage == Page.NormalTabs && normalTabCount == 0 || - selectedPage == Page.PrivateTabs && privateTabCount == 0 -> listOf( - tabSettingsItem, - recentlyClosedTabsItem, - ) - - selectedPage == Page.NormalTabs -> listOf( - enterSelectModeItem, - shareAllTabsItem, - tabSettingsItem, - recentlyClosedTabsItem, - deleteAllTabsItem, - ) - - selectedPage == Page.PrivateTabs -> listOf( - tabSettingsItem, - recentlyClosedTabsItem, - deleteAllTabsItem, - ) - - selectedPage == Page.SyncedTabs -> listOf( - accountSettingsItem, - recentlyClosedTabsItem, - ) - - else -> emptyList() - } -} - private const val MAX_VISIBLE_TABS = 99 private const val SO_MANY_TABS_OPEN = "∞" private val NORMAL_TABS_BOTTOM_PADDING = 0.5.dp @@ -453,46 +375,22 @@ private fun NormalTabsTabIcon(normalTabCount: Int) { /** * Banner displayed in multi select mode. * + * @param menuItems List of items in the menu. * @param selectedTabCount Number of selected tabs. - * @param shouldShowInactiveButton Whether or not to show the inactive tabs menu item. * @param onExitSelectModeClick Invoked when the user clicks on exit select mode button. * @param onSaveToCollectionsClick Invoked when the user clicks on the save to collection button. * @param onShareSelectedTabs Invoked when the user clicks on the share button. - * @param onBookmarkSelectedTabsClick Invoked when user interacts with the bookmark menu item. - * @param onCloseSelectedTabsClick Invoked when user interacts with the close menu item. - * @param onMakeSelectedTabsInactive Invoked when user interacts with the make inactive menu item. */ @Suppress("LongMethod", "LongParameterList") @Composable private fun MultiSelectBanner( + menuItems: List, selectedTabCount: Int, - shouldShowInactiveButton: Boolean, onExitSelectModeClick: () -> Unit, onSaveToCollectionsClick: () -> Unit, onShareSelectedTabs: () -> Unit, - onBookmarkSelectedTabsClick: () -> Unit, - onCloseSelectedTabsClick: () -> Unit, - onMakeSelectedTabsInactive: () -> Unit, ) { var showMenu by remember { mutableStateOf(false) } - val menuItems = mutableListOf( - MenuItem( - title = stringResource(R.string.tab_tray_multiselect_menu_item_bookmark), - onClick = onBookmarkSelectedTabsClick, - ), - MenuItem( - title = stringResource(R.string.tab_tray_multiselect_menu_item_close), - onClick = onCloseSelectedTabsClick, - ), - ) - if (shouldShowInactiveButton) { - menuItems.add( - MenuItem( - title = stringResource(R.string.inactive_tabs_menu_item), - onClick = onMakeSelectedTabsInactive, - ), - ) - } Row( modifier = Modifier diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabsTrayState.kt b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabsTrayState.kt index 96434cc5dc..50edcba44f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabsTrayState.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabsTrayState.kt @@ -4,9 +4,181 @@ package org.mozilla.fenix.tabstray.ext +import android.content.res.Resources +import org.mozilla.fenix.R +import org.mozilla.fenix.compose.MenuItem +import org.mozilla.fenix.tabstray.Page import org.mozilla.fenix.tabstray.TabsTrayState.Mode +import org.mozilla.fenix.tabstray.TabsTrayTestTag /** * A helper to check if we're in [Mode.Select] mode. */ fun Mode.isSelect() = this is Mode.Select + +/** + * Returns the list of menu items corresponding to the selected mode + * + * @param resources The resources used to provide the strings for the menu item titles. + * @param shouldShowInactiveButton Whether or not to show the inactive tabs menu item. + * @param selectedPage The currently selected page. + * @param normalTabCount The normal tabs number. + * @param privateTabCount The private tabs number. + * @param onBookmarkSelectedTabsClick Invoked when user interacts with the bookmark menu item. + * @param onCloseSelectedTabsClick Invoked when user interacts with the close menu item. + * @param onMakeSelectedTabsInactive Invoked when user interacts with the make inactive menu item. + * @param onTabSettingsClick Invoked when user interacts with the tab settings menu. + * @param onRecentlyClosedClick Invoked when user interacts with the recently closed menu item. + * @param onEnterMultiselectModeClick Invoked when user enters the multiselect mode. + * @param onShareAllTabsClick Invoked when user interacts with the share all menu item. + * @param onDeleteAllTabsClick Invoked when user interacts with the delete all menu item. + * @param onAccountSettingsClick Invoked when user interacts with the account settings. + */ +@Suppress("LongParameterList") +fun Mode.getMenuItems( + resources: Resources, + shouldShowInactiveButton: Boolean, + selectedPage: Page, + normalTabCount: Int, + privateTabCount: Int, + onBookmarkSelectedTabsClick: () -> Unit, + onCloseSelectedTabsClick: () -> Unit, + onMakeSelectedTabsInactive: () -> Unit, + onTabSettingsClick: () -> Unit, + onRecentlyClosedClick: () -> Unit, + onEnterMultiselectModeClick: () -> Unit, + onShareAllTabsClick: () -> Unit, + onDeleteAllTabsClick: () -> Unit, + onAccountSettingsClick: () -> Unit, +): List { + return if (this.isSelect()) { + generateMultiSelectBannerMenuItems( + resources = resources, + shouldShowInactiveButton = shouldShowInactiveButton, + onBookmarkSelectedTabsClick = onBookmarkSelectedTabsClick, + onCloseSelectedTabsClick = onCloseSelectedTabsClick, + onMakeSelectedTabsInactive = onMakeSelectedTabsInactive, + ) + } else { + generateSingleSelectBannerMenuItems( + resources = resources, + selectedPage = selectedPage, + normalTabCount = normalTabCount, + privateTabCount = privateTabCount, + onTabSettingsClick = onTabSettingsClick, + onRecentlyClosedClick = onRecentlyClosedClick, + onEnterMultiselectModeClick = onEnterMultiselectModeClick, + onShareAllTabsClick = onShareAllTabsClick, + onDeleteAllTabsClick = onDeleteAllTabsClick, + onAccountSettingsClick = onAccountSettingsClick, + ) + } +} + +/** + * Builds the menu items list when in multiselect mode + */ +private fun generateMultiSelectBannerMenuItems( + resources: Resources, + shouldShowInactiveButton: Boolean, + onBookmarkSelectedTabsClick: () -> Unit, + onCloseSelectedTabsClick: () -> Unit, + onMakeSelectedTabsInactive: () -> Unit, +): List { + val menuItems = mutableListOf( + MenuItem( + title = resources.getString(R.string.tab_tray_multiselect_menu_item_bookmark), + onClick = onBookmarkSelectedTabsClick, + ), + MenuItem( + title = resources.getString(R.string.tab_tray_multiselect_menu_item_close), + onClick = onCloseSelectedTabsClick, + ), + ) + if (shouldShowInactiveButton) { + menuItems.add( + MenuItem( + title = resources.getString(R.string.inactive_tabs_menu_item), + onClick = onMakeSelectedTabsInactive, + ), + ) + } + + return menuItems +} + +/** + * Builds the menu items list when in normal mode + */ +@Suppress("LongParameterList") +private fun generateSingleSelectBannerMenuItems( + resources: Resources, + selectedPage: Page, + normalTabCount: Int, + privateTabCount: Int, + onTabSettingsClick: () -> Unit, + onRecentlyClosedClick: () -> Unit, + onEnterMultiselectModeClick: () -> Unit, + onShareAllTabsClick: () -> Unit, + onDeleteAllTabsClick: () -> Unit, + onAccountSettingsClick: () -> Unit, +): List { + val tabSettingsItem = MenuItem( + title = resources.getString(R.string.tab_tray_menu_tab_settings), + testTag = TabsTrayTestTag.tabSettings, + onClick = onTabSettingsClick, + ) + val recentlyClosedTabsItem = MenuItem( + title = resources.getString(R.string.tab_tray_menu_recently_closed), + testTag = TabsTrayTestTag.recentlyClosedTabs, + onClick = onRecentlyClosedClick, + ) + val enterSelectModeItem = MenuItem( + title = resources.getString(R.string.tabs_tray_select_tabs), + testTag = TabsTrayTestTag.selectTabs, + onClick = onEnterMultiselectModeClick, + ) + val shareAllTabsItem = MenuItem( + title = resources.getString(R.string.tab_tray_menu_item_share), + testTag = TabsTrayTestTag.shareAllTabs, + onClick = onShareAllTabsClick, + ) + val deleteAllTabsItem = MenuItem( + title = resources.getString(R.string.tab_tray_menu_item_close), + testTag = TabsTrayTestTag.closeAllTabs, + onClick = onDeleteAllTabsClick, + ) + val accountSettingsItem = MenuItem( + title = resources.getString(R.string.tab_tray_menu_account_settings), + testTag = TabsTrayTestTag.accountSettings, + onClick = onAccountSettingsClick, + ) + return when { + selectedPage == Page.NormalTabs && normalTabCount == 0 || + selectedPage == Page.PrivateTabs && privateTabCount == 0 -> listOf( + tabSettingsItem, + recentlyClosedTabsItem, + ) + + selectedPage == Page.NormalTabs -> listOf( + enterSelectModeItem, + shareAllTabsItem, + tabSettingsItem, + recentlyClosedTabsItem, + deleteAllTabsItem, + ) + + selectedPage == Page.PrivateTabs -> listOf( + tabSettingsItem, + recentlyClosedTabsItem, + deleteAllTabsItem, + ) + + selectedPage == Page.SyncedTabs -> listOf( + accountSettingsItem, + recentlyClosedTabsItem, + ) + + else -> emptyList() + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStateTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStateTest.kt new file mode 100644 index 0000000000..e146074b3d --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStateTest.kt @@ -0,0 +1,206 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.compose.MenuItem +import org.mozilla.fenix.tabstray.ext.getMenuItems +import org.mozilla.fenix.tabstray.ext.isSelect + +@RunWith(AndroidJUnit4::class) +class TabsTrayStateTest { + + private lateinit var store: TabsTrayStore + + @Before + fun setup() { + store = TabsTrayStore() + } + + @Test + fun `WHEN entering select mode THEN isSelected extension method returns true`() { + store.dispatch(TabsTrayAction.EnterSelectMode) + store.waitUntilIdle() + + Assert.assertTrue(store.state.mode.isSelect()) + } + + @Test + fun `WHEN entering normal mode THEN isSelected extension method returns false`() { + store.dispatch(TabsTrayAction.ExitSelectMode) + store.waitUntilIdle() + + Assert.assertFalse(store.state.mode.isSelect()) + } + + @Test + fun `GIVEN select mode is selected and show the inactive button is true WHEN entering any page THEN return 3 items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Select(emptySet()), + shouldShowInactiveButton = true, + ) + Assert.assertEquals(menuItems.size, 3) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_multiselect_menu_item_bookmark), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_multiselect_menu_item_close), + ) + Assert.assertEquals( + menuItems[2].title, + testContext.getString(R.string.inactive_tabs_menu_item), + ) + } + + @Test + fun `GIVEN select mode is selected and show the inactive button is false WHEN entering any page THEN return 2 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Select(emptySet()), + ) + Assert.assertEquals(menuItems.size, 2) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_multiselect_menu_item_bookmark), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_multiselect_menu_item_close), + ) + } + + @Test + fun `GIVEN normal mode is selected and no normal tabs are opened WHEN entering normal page THEN return 2 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Normal, + ) + Assert.assertEquals(menuItems.size, 2) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_menu_tab_settings), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_menu_recently_closed), + ) + } + + @Test + fun `GIVEN normal mode is selected and multiple normal tabs are opened WHEN entering normal page THEN return 5 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Normal, + normalTabCount = 3, + ) + Assert.assertEquals(menuItems.size, 5) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tabs_tray_select_tabs), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_menu_item_share), + ) + Assert.assertEquals( + menuItems[2].title, + testContext.getString(R.string.tab_tray_menu_tab_settings), + ) + Assert.assertEquals( + menuItems[3].title, + testContext.getString(R.string.tab_tray_menu_recently_closed), + ) + Assert.assertEquals( + menuItems[4].title, + testContext.getString(R.string.tab_tray_menu_item_close), + ) + } + + @Test + fun `GIVEN normal mode is selected and no private tabs are opened WHEN entering private page THEN return 2 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Normal, + selectedPage = Page.PrivateTabs, + ) + Assert.assertEquals(menuItems.size, 2) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_menu_tab_settings), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_menu_recently_closed), + ) + } + + @Test + fun `GIVEN normal mode is selected and multiple private tabs are opened WHEN entering private page THEN return 3 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Normal, + selectedPage = Page.PrivateTabs, + privateTabCount = 6, + ) + Assert.assertEquals(menuItems.size, 3) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_menu_tab_settings), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_menu_recently_closed), + ) + Assert.assertEquals( + menuItems[2].title, + testContext.getString(R.string.tab_tray_menu_item_close), + ) + } + + @Test + fun `GIVEN normal mode is selected WHEN entering synced page THEN return 2 menu items`() { + val menuItems = initMenuItems( + mode = TabsTrayState.Mode.Normal, + selectedPage = Page.SyncedTabs, + ) + Assert.assertEquals(menuItems.size, 2) + Assert.assertEquals( + menuItems[0].title, + testContext.getString(R.string.tab_tray_menu_account_settings), + ) + Assert.assertEquals( + menuItems[1].title, + testContext.getString(R.string.tab_tray_menu_recently_closed), + ) + } + + private fun initMenuItems( + mode: TabsTrayState.Mode, + shouldShowInactiveButton: Boolean = false, + selectedPage: Page = Page.NormalTabs, + normalTabCount: Int = 0, + privateTabCount: Int = 0, + ): List = + mode.getMenuItems( + resources = testContext.resources, + shouldShowInactiveButton = shouldShowInactiveButton, + selectedPage = selectedPage, + normalTabCount = normalTabCount, + privateTabCount = privateTabCount, + onBookmarkSelectedTabsClick = {}, + onCloseSelectedTabsClick = {}, + onMakeSelectedTabsInactive = {}, + onTabSettingsClick = {}, + onRecentlyClosedClick = {}, + onEnterMultiselectModeClick = {}, + onShareAllTabsClick = {}, + onDeleteAllTabsClick = {}, + onAccountSettingsClick = {}, + ) +}