From 061de542911f0798f5dda85c52a316117f5d2107 Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Sat, 2 Oct 2021 03:14:01 -0400 Subject: [PATCH] Issue #21576: Hide 'Other' title when there are no search groups --- .../org/mozilla/fenix/ext/BrowserState.kt | 2 +- .../tabstray/browser/TitleHeaderBinding.kt | 16 +++---- .../fenix/tabstray/ext/TabSelectors.kt | 13 +++++ .../browser/TitleHeaderBindingTest.kt | 48 ++++++++++++++++++- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt index 0e01cec0c4..b3fc5016c3 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt @@ -82,7 +82,7 @@ val BrowserState.lastSearchGroup: RecentTab.SearchGroup? /** * Get search term groups sorted by last access time. */ -private fun List.toSearchGroup(): List { +fun List.toSearchGroup(): List { val data = filter { it.isNormalTabActiveWithSearchTerm(maxActiveTime) }.groupBy { diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt index 163ca067e5..7a4b0514b8 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt @@ -13,6 +13,7 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.lib.state.helpers.AbstractBinding import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.tabstray.ext.getNormalTrayTabs +import org.mozilla.fenix.tabstray.ext.getSearchTabGroups import org.mozilla.fenix.utils.Settings /** @@ -25,14 +26,13 @@ class TitleHeaderBinding( private val showHeader: (Boolean) -> Unit ) : AbstractBinding(store) { override suspend fun onState(flow: Flow) { - flow.map { - it.getNormalTrayTabs( - settings.searchTermTabGroupsAreEnabled, - settings.inactiveTabsAreEnabled - ) - }.ifChanged { it.size } - .collect { - if (it.isEmpty()) { + val groupsEnabled = settings.searchTermTabGroupsAreEnabled + val inactiveEnabled = settings.inactiveTabsAreEnabled + + flow.map { it.getSearchTabGroups(groupsEnabled) to it.getNormalTrayTabs(groupsEnabled, inactiveEnabled) } + .ifChanged() + .collect { (groups, normalTrayTabs) -> + if (groups.isEmpty() || normalTrayTabs.isEmpty()) { showHeader(false) } else { showHeader(true) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt index 0cac457365..a1a5d3d7b0 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt @@ -8,6 +8,8 @@ import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.TabSessionState +import org.mozilla.fenix.ext.toSearchGroup +import org.mozilla.fenix.tabstray.browser.TabGroup import org.mozilla.fenix.tabstray.browser.maxActiveTime /** @@ -56,3 +58,14 @@ fun BrowserState.getNormalTrayTabs( } } } + +/** + * The list of search groups filtered appropriately based on feature flags. + */ +fun BrowserState.getSearchTabGroups( + searchTermTabGroupsAreEnabled: Boolean +): List = if (searchTermTabGroupsAreEnabled) { + normalTabs.toSearchGroup().filter { it.tabs.size > 1 } +} else { + emptyList() +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBindingTest.kt index 1fcb1c47ea..68c283b689 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBindingTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBindingTest.kt @@ -29,7 +29,7 @@ class TitleHeaderBindingTest { val coroutinesTestRule = MainCoroutineRule() @Test - fun `WHEN normal tabs are added to the list THEN return true`() = runBlockingTest { + fun `WHEN normal tabs are added to the list THEN return false`() = runBlockingTest { var result = false val store = BrowserStore() val settings: Settings = mockk(relaxed = true) @@ -43,11 +43,55 @@ class TitleHeaderBindingTest { store.waitUntilIdle() + assertFalse(result) + } + + @Test + fun `WHEN grouped and normal tabs are added THEN return true`() = runBlockingTest { + var result = false + val store = BrowserStore( + initialState = BrowserState( + listOf( + createTab( + url = "https://mozilla.org", + historyMetadata = HistoryMetadataKey( + url = "https://getpocket.com", + searchTerm = "Mozilla" + ) + ), + createTab( + url = "https://firefox.com", + historyMetadata = HistoryMetadataKey( + url = "https://getpocket.com", + searchTerm = "Mozilla" + ) + ) + ) + ) + ) + val settings: Settings = mockk(relaxed = true) + val binding = TitleHeaderBinding(store, settings) { result = it } + + every { settings.inactiveTabsAreEnabled } returns true + every { settings.searchTermTabGroupsAreEnabled } returns true + + binding.start() + + store.dispatch( + TabListAction.AddTabAction( + createTab( + url = "https://getpocket.com", + ) + ) + ).joinBlocking() + + store.waitUntilIdle() + assertTrue(result) } @Test - fun `WHEN grouped tabs are added to the list THEN return false`() = runBlockingTest { + fun `WHEN grouped tab is added to the list THEN return false`() = runBlockingTest { var result = false val store = BrowserStore() val settings: Settings = mockk(relaxed = true)