From 4b3f89f810e3dcd1bf97c2d75b7fd70fe78d237d Mon Sep 17 00:00:00 2001 From: Noah Bond Date: Wed, 22 Mar 2023 14:05:46 -0700 Subject: [PATCH] Bug 1821206 - Implement composified tab list click listeners --- .../org/mozilla/fenix/tabstray/TabsTray.kt | 48 ++++++++++++++++++- .../fenix/tabstray/TabsTrayFragment.kt | 47 +++++++++++------- .../fenix/tabstray/TabsTrayTabLayouts.kt | 40 ++++++++++++---- .../fenix/tabstray/TabsTrayFragmentTest.kt | 4 ++ 4 files changed, 114 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTray.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTray.kt index 2e7e5c2d48..550478b8d0 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTray.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTray.kt @@ -33,12 +33,23 @@ import org.mozilla.fenix.theme.FirefoxTheme * * @param tabsTrayStore [TabsTrayStore] used to listen for changes to [TabsTrayState]. * @param displayTabsInGrid Whether the normal and private tabs should be displayed in a grid. + * @param onTabClose Invoked when the user clicks to close a tab. + * @param onTabMediaClick Invoked when the user interacts with a tab's media controls. + * @param onTabClick Invoked when the user clicks on a tab. + * @param onTabMultiSelectClick Invoked when the user clicks on a tab while in multi-select mode. + * @param onTabLongClick Invoked when the user long clicks a tab. */ @OptIn(ExperimentalPagerApi::class, ExperimentalComposeUiApi::class) +@Suppress("LongMethod", "LongParameterList") @Composable fun TabsTray( tabsTrayStore: TabsTrayStore, displayTabsInGrid: Boolean, + onTabClose: (TabSessionState) -> Unit, + onTabMediaClick: (TabSessionState) -> Unit, + onTabClick: (TabSessionState) -> Unit, + onTabMultiSelectClick: (TabSessionState) -> Unit, + onTabLongClick: (TabSessionState) -> Unit, ) { val multiselectMode = tabsTrayStore .observeAsComposableState { state -> state.mode }.value ?: TabsTrayState.Mode.Normal @@ -48,11 +59,20 @@ fun TabsTray( .observeAsComposableState { state -> state.privateTabs }.value ?: emptyList() val pagerState = rememberPagerState(initialPage = 0) val scope = rememberCoroutineScope() + val isInMultiSelectMode = multiselectMode is TabsTrayState.Mode.Select + val animateScrollToPage: ((Page) -> Unit) = { page -> scope.launch { pagerState.animateScrollToPage(page.ordinal) } } + val handleTabClick: ((TabSessionState) -> Unit) = { tab -> + if (isInMultiSelectMode) { + onTabMultiSelectClick(tab) + } else { + onTabClick(tab) + } + } Column( modifier = Modifier @@ -61,7 +81,7 @@ fun TabsTray( ) { Box(modifier = Modifier.nestedScroll(rememberNestedScrollInteropConnection())) { TabsTrayBanner( - isInMultiSelectMode = multiselectMode is TabsTrayState.Mode.Select, + isInMultiSelectMode = isInMultiSelectMode, onTabPageIndicatorClicked = animateScrollToPage, ) } @@ -80,10 +100,18 @@ fun TabsTray( if (displayTabsInGrid) { TabGrid( tabs = normalTabs, + onTabClose = onTabClose, + onTabMediaClick = onTabMediaClick, + onTabClick = handleTabClick, + onTabLongClick = onTabLongClick, ) } else { TabList( tabs = normalTabs, + onTabClose = onTabClose, + onTabMediaClick = onTabMediaClick, + onTabClick = handleTabClick, + onTabLongClick = onTabLongClick, ) } } @@ -91,10 +119,18 @@ fun TabsTray( if (displayTabsInGrid) { TabGrid( tabs = privateTabs, + onTabClose = onTabClose, + onTabMediaClick = onTabMediaClick, + onTabClick = handleTabClick, + onTabLongClick = onTabLongClick, ) } else { TabList( tabs = privateTabs, + onTabClose = onTabClose, + onTabMediaClick = onTabMediaClick, + onTabClick = handleTabClick, + onTabLongClick = onTabLongClick, ) } } @@ -129,6 +165,11 @@ private fun TabsTrayPreview() { TabsTray( tabsTrayStore = store, displayTabsInGrid = false, + onTabClose = {}, + onTabMediaClick = {}, + onTabClick = {}, + onTabMultiSelectClick = {}, + onTabLongClick = {}, ) } } @@ -147,6 +188,11 @@ private fun TabsTrayMultiSelectPreview() { TabsTray( tabsTrayStore = store, displayTabsInGrid = true, + onTabClose = {}, + onTabMediaClick = {}, + onTabClick = {}, + onTabMultiSelectClick = {}, + onTabLongClick = {}, ) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt index 20f13e4304..3a0085b8d4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -210,6 +210,17 @@ class TabsTrayFragment : AppCompatDialogFragment() { TabsTray( tabsTrayStore = tabsTrayStore, displayTabsInGrid = requireContext().settings().gridTabView, + onTabClose = { tab -> + tabsTrayInteractor.onTabClosed(tab, TABS_TRAY_FEATURE_NAME) + }, + onTabMediaClick = tabsTrayInteractor::onMediaClicked, + onTabClick = { tab -> + tabsTrayInteractor.onTabSelected(tab, TABS_TRAY_FEATURE_NAME) + }, + onTabMultiSelectClick = { tab -> + tabsTrayInteractor.onMultiSelectClicked(tab, TABS_TRAY_FEATURE_NAME) + }, + onTabLongClick = tabsTrayInteractor::onTabLongClicked, ) } } @@ -484,22 +495,24 @@ class TabsTrayFragment : AppCompatDialogFragment() { false -> getString(R.string.snackbar_tab_closed) } - lifecycleScope.allowUndo( - requireView(), - snackbarMessage, - getString(R.string.snackbar_deleted_undo), - { - requireComponents.useCases.tabsUseCases.undo.invoke() - tabLayoutMediator.withFeature { - it.selectTabAtPosition( - if (isPrivate) Page.PrivateTabs.ordinal else Page.NormalTabs.ordinal, - ) - } - }, - operation = { }, - elevation = ELEVATION, - anchorView = if (fabButtonBinding.newTabButton.isVisible) fabButtonBinding.newTabButton else null, - ) + if (!requireContext().settings().enableTabsTrayToCompose) { + lifecycleScope.allowUndo( + requireView(), + snackbarMessage, + getString(R.string.snackbar_deleted_undo), + { + requireComponents.useCases.tabsUseCases.undo.invoke() + tabLayoutMediator.withFeature { + it.selectTabAtPosition( + if (isPrivate) Page.PrivateTabs.ordinal else Page.NormalTabs.ordinal, + ) + } + }, + operation = { }, + elevation = ELEVATION, + anchorView = if (fabButtonBinding.newTabButton.isVisible) fabButtonBinding.newTabButton else null, + ) + } } @VisibleForTesting @@ -646,5 +659,7 @@ class TabsTrayFragment : AppCompatDialogFragment() { // Elevation for undo toasts @VisibleForTesting internal const val ELEVATION = 80f + + private const val TABS_TRAY_FEATURE_NAME = "Tabs tray" } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayTabLayouts.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayTabLayouts.kt index f21808d2c1..d41f6a431f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayTabLayouts.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayTabLayouts.kt @@ -34,10 +34,18 @@ import org.mozilla.fenix.theme.FirefoxTheme * Top-level UI for displaying a list of tabs. * * @param tabs The list of [TabSessionState] to display. + * @param onTabClose Invoked when the user clicks to close a tab. + * @param onTabMediaClick Invoked when the user interacts with a tab's media controls. + * @param onTabClick Invoked when the user clicks on a tab. + * @param onTabLongClick Invoked when the user long clicks a tab. */ @Composable fun TabList( tabs: List, + onTabClose: (TabSessionState) -> Unit, + onTabMediaClick: (TabSessionState) -> Unit, + onTabClick: (TabSessionState) -> Unit, + onTabLongClick: (TabSessionState) -> Unit, ) { val tabListBottomPadding = dimensionResource(id = R.dimen.tab_tray_list_bottom_padding) val state = rememberLazyListState() @@ -52,10 +60,10 @@ fun TabList( ) { tab -> TabListItem( tab = tab, - onCloseClick = {}, - onMediaClick = {}, - onClick = {}, - onLongClick = {}, + onCloseClick = onTabClose, + onMediaClick = onTabMediaClick, + onClick = onTabClick, + onLongClick = onTabLongClick, ) } @@ -69,10 +77,18 @@ fun TabList( * Top-level UI for displaying a grid of tabs. * * @param tabs The list of [TabSessionState] to display. + * @param onTabClose Invoked when the user clicks to close a tab. + * @param onTabMediaClick Invoked when the user interacts with a tab's media controls. + * @param onTabClick Invoked when the user clicks on a tab. + * @param onTabLongClick Invoked when the user long clicks a tab. */ @Composable fun TabGrid( tabs: List, + onTabClose: (tab: TabSessionState) -> Unit, + onTabMediaClick: (tab: TabSessionState) -> Unit, + onTabClick: (tab: TabSessionState) -> Unit, + onTabLongClick: (tab: TabSessionState) -> Unit, ) { val tabListBottomPadding = dimensionResource(id = R.dimen.tab_tray_list_bottom_padding) val state = rememberLazyGridState() @@ -88,10 +104,10 @@ fun TabGrid( ) { tab -> TabGridItem( tab = tab, - onCloseClick = {}, - onMediaClick = {}, - onClick = {}, - onLongClick = {}, + onCloseClick = onTabClose, + onMediaClick = onTabMediaClick, + onClick = onTabClick, + onLongClick = onTabLongClick, ) } @@ -112,6 +128,10 @@ private fun TabListPreview() { ) { TabList( tabs = generateFakeTabsList(), + onTabClose = {}, + onTabMediaClick = {}, + onTabClick = {}, + onTabLongClick = {}, ) } } @@ -128,6 +148,10 @@ private fun TabGridPreview() { ) { TabGrid( tabs = generateFakeTabsList(), + onTabClose = {}, + onTabMediaClick = {}, + onTabClick = {}, + onTabLongClick = {}, ) } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt index cb46a6e704..22d24be8e4 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayFragmentTest.kt @@ -100,6 +100,7 @@ class TabsTrayFragmentTest { every { fragment.context } returns testContext // needed for getString() every { any().allowUndo(any(), any(), any(), any(), any(), any(), any(), any()) } just Runs every { fragment.requireView() } returns view + every { testContext.settings().enableTabsTrayToCompose } returns false fragment.showUndoSnackbarForTab(true) @@ -131,6 +132,7 @@ class TabsTrayFragmentTest { every { fragment.context } returns testContext // needed for getString() every { any().allowUndo(any(), any(), any(), any(), any(), any(), any(), any()) } just Runs every { fragment.requireView() } returns view + every { testContext.settings().enableTabsTrayToCompose } returns false fragment.showUndoSnackbarForTab(true) @@ -163,6 +165,7 @@ class TabsTrayFragmentTest { every { fragment.context } returns testContext // needed for getString() every { any().allowUndo(any(), any(), any(), any(), any(), any(), any(), any()) } just Runs every { fragment.requireView() } returns view + every { testContext.settings().enableTabsTrayToCompose } returns false fragment.showUndoSnackbarForTab(false) @@ -194,6 +197,7 @@ class TabsTrayFragmentTest { every { fragment.context } returns testContext // needed for getString() every { any().allowUndo(any(), any(), any(), any(), any(), any(), any(), any()) } just Runs every { fragment.requireView() } returns view + every { testContext.settings().enableTabsTrayToCompose } returns false fragment.showUndoSnackbarForTab(false)