diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt index e33b709c2..b54aeea84 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt @@ -42,7 +42,7 @@ class TrayPagerAdapter( ConcatAdapter( InactiveTabsAdapter(context, browserInteractor, interactor, INACTIVE_TABS_FEATURE_NAME, context.settings()), TabGroupAdapter(context, browserInteractor, tabsTrayStore, TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(browserStore, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, browserInteractor, tabsTrayStore, TABS_TRAY_FEATURE_NAME) ) } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt index ff70e122f..bdbef6b85 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt @@ -17,6 +17,7 @@ import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter import org.mozilla.fenix.tabstray.ext.tabGroupAdapter +import org.mozilla.fenix.tabstray.ext.titleHeaderAdapter import org.mozilla.fenix.utils.Settings import kotlin.math.max @@ -57,6 +58,10 @@ class TabSorter( val selectedTabIndex = totalNormalTabs.findSelectedIndex(selectedTabId) concatAdapter.browserAdapter.updateTabs(Tabs(totalNormalTabs, selectedTabIndex)) + // Normal tab title header. + concatAdapter.titleHeaderAdapter + .handleListChanges(totalNormalTabs.isNotEmpty() && groups.isNotEmpty()) + if (shouldReportMetrics) { shouldReportMetrics = false diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderAdapter.kt index 0b421a694..bbd3f3347 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderAdapter.kt @@ -10,23 +10,17 @@ import android.view.ViewGroup import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter import androidx.recyclerview.widget.RecyclerView -import mozilla.components.browser.state.store.BrowserStore import org.mozilla.fenix.R import org.mozilla.fenix.databinding.TabTrayTitleHeaderItemBinding -import org.mozilla.fenix.utils.Settings /** * A [RecyclerView.Adapter] for tab header. */ -class TitleHeaderAdapter( - browserStore: BrowserStore, - settings: Settings -) : ListAdapter(DiffCallback) { +class TitleHeaderAdapter : + ListAdapter(DiffCallback) { class Header - private val normalTabsHeaderBinding = TitleHeaderBinding(browserStore, settings, ::handleListChanges) - override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): HeaderViewHolder { val view = LayoutInflater.from(parent.context).inflate(viewType, parent, false) return HeaderViewHolder(view) @@ -34,18 +28,10 @@ class TitleHeaderAdapter( override fun getItemViewType(position: Int) = HeaderViewHolder.LAYOUT_ID - override fun onAttachedToRecyclerView(recyclerView: RecyclerView) { - normalTabsHeaderBinding.start() - } - - override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) { - normalTabsHeaderBinding.stop() - } - /* Do nothing */ override fun onBindViewHolder(holder: HeaderViewHolder, position: Int) = Unit - private fun handleListChanges(showHeader: Boolean) { + fun handleListChanges(showHeader: Boolean) { val header = if (showHeader) { listOf(Header()) } else { 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 deleted file mode 100644 index 7a4b0514b..000000000 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBinding.kt +++ /dev/null @@ -1,42 +0,0 @@ -/* 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.browser - -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.collect -import kotlinx.coroutines.flow.map -import mozilla.components.browser.state.state.BrowserState -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 - -/** - * A binding class to notify an observer to show a title if there is at least one tab available. - */ -@OptIn(ExperimentalCoroutinesApi::class) -class TitleHeaderBinding( - store: BrowserStore, - private val settings: Settings, - private val showHeader: (Boolean) -> Unit -) : AbstractBinding(store) { - override suspend fun onState(flow: Flow) { - 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 2f6804987..8a93a7800 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 @@ -9,7 +9,6 @@ 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 /** @@ -60,14 +59,3 @@ fun BrowserState.getNormalTrayTabs( } } } - -/** - * The list of search groups filtered appropriately based on feature flags. - */ -fun BrowserState.getSearchTabGroups( - searchTermTabGroupsAreEnabled: Boolean -): List = if (searchTermTabGroupsAreEnabled) { - normalTabs.toSearchGroup().first -} else { - emptyList() -} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt index ed4b91dfd..b359433a9 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt @@ -17,7 +17,6 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.INACTIVE_TABS_FEATURE_NAME import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.TABS_TRAY_FEATURE_NAME @@ -52,7 +51,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -83,7 +82,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -99,9 +98,10 @@ class TabSorterTest { ) ) - assertEquals(adapter.itemCount, 2) + assertEquals(adapter.itemCount, 3) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 1) + assertEquals(adapter.titleHeaderAdapter.itemCount, 1) assertEquals(adapter.browserAdapter.itemCount, 1) } @@ -115,7 +115,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -132,9 +132,10 @@ class TabSorterTest { ) ) - assertEquals(adapter.itemCount, 3) + assertEquals(adapter.itemCount, 4) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 1) assertEquals(adapter.tabGroupAdapter.itemCount, 1) + assertEquals(adapter.titleHeaderAdapter.itemCount, 1) assertEquals(adapter.browserAdapter.itemCount, 1) } @@ -149,7 +150,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -166,9 +167,10 @@ class TabSorterTest { ) ) - assertEquals(adapter.itemCount, 3) + assertEquals(adapter.itemCount, 4) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 1) + assertEquals(adapter.titleHeaderAdapter.itemCount, 1) assertEquals(adapter.browserAdapter.itemCount, 2) } @@ -183,7 +185,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -203,6 +205,7 @@ class TabSorterTest { assertEquals(adapter.itemCount, 4) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 1) assertEquals(adapter.tabGroupAdapter.itemCount, 0) + assertEquals(adapter.titleHeaderAdapter.itemCount, 0) assertEquals(adapter.browserAdapter.itemCount, 3) } @@ -218,7 +221,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -238,6 +241,7 @@ class TabSorterTest { assertEquals(adapter.itemCount, 4) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 0) + assertEquals(adapter.titleHeaderAdapter.itemCount, 0) assertEquals(adapter.browserAdapter.itemCount, 4) } @@ -251,7 +255,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -268,6 +272,7 @@ class TabSorterTest { assertEquals(adapter.itemCount, 1) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 0) + assertEquals(adapter.titleHeaderAdapter.itemCount, 0) assertEquals(adapter.browserAdapter.itemCount, 1) } @@ -281,7 +286,7 @@ class TabSorterTest { val adapter = ConcatAdapter( InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings), TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME), - TitleHeaderAdapter(store, context.settings()), + TitleHeaderAdapter(), BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME) ) val tabSorter = TabSorter(settings, metrics, adapter, store) @@ -299,6 +304,7 @@ class TabSorterTest { assertEquals(adapter.itemCount, 1) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 1) + assertEquals(adapter.titleHeaderAdapter.itemCount, 0) assertEquals(adapter.browserAdapter.itemCount, 0) tabSorter.updateTabs( @@ -313,6 +319,7 @@ class TabSorterTest { assertEquals(adapter.itemCount, 1) assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0) assertEquals(adapter.tabGroupAdapter.itemCount, 1) + assertEquals(adapter.titleHeaderAdapter.itemCount, 0) assertEquals(adapter.browserAdapter.itemCount, 0) } } 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 deleted file mode 100644 index 68c283b68..000000000 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TitleHeaderBindingTest.kt +++ /dev/null @@ -1,141 +0,0 @@ -/* 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.browser - -import io.mockk.every -import io.mockk.mockk -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.state.action.TabListAction -import mozilla.components.browser.state.state.BrowserState -import mozilla.components.browser.state.state.createTab -import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.concept.storage.HistoryMetadataKey -import mozilla.components.support.test.ext.joinBlocking -import mozilla.components.support.test.libstate.ext.waitUntilIdle -import mozilla.components.support.test.rule.MainCoroutineRule -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Rule -import org.junit.Test -import org.mozilla.fenix.utils.Settings - -@ExperimentalCoroutinesApi -class TitleHeaderBindingTest { - - @get:Rule - val coroutinesTestRule = MainCoroutineRule() - - @Test - 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) - val binding = TitleHeaderBinding(store, settings) { result = it } - - every { settings.inactiveTabsAreEnabled } returns true - - binding.start() - - store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))).joinBlocking() - - 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 tab is added to the list THEN return false`() = runBlockingTest { - var result = false - val store = BrowserStore() - 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://mozilla.org", - historyMetadata = HistoryMetadataKey( - url = "https://getpocket.com", - searchTerm = "Mozilla" - ) - ) - ) - ).joinBlocking() - - store.waitUntilIdle() - - assertFalse(result) - } - - @Test - fun `WHEN normal tabs are all removed THEN return false`() = runBlockingTest { - var result = false - val store = BrowserStore( - initialState = BrowserState( - tabs = listOf(createTab("https://getpocket.com", id = "123")) - ) - ) - val settings: Settings = mockk(relaxed = true) - val binding = TitleHeaderBinding(store, settings) { result = it } - - binding.start() - - store.dispatch(TabListAction.RemoveTabAction("123")).joinBlocking() - - store.waitUntilIdle() - - assertFalse(result) - } -}