Issue #21893: Keep search group when second to last search term tab is removed

upstream-sync
Roger Yang 3 years ago committed by mergify[bot]
parent 31a0d676ee
commit 60f7f766da

@ -38,7 +38,7 @@ class NormalBrowserTrayList @JvmOverloads constructor(
private val swipeDelegate = SwipeToDeleteDelegate()
private val concatAdapter by lazy { adapter as ConcatAdapter }
private val tabSorter by lazy { TabSorter(context, concatAdapter, context.components.core.store) }
private val tabSorter by lazy { TabSorter(context.settings(), concatAdapter, context.components.core.store) }
private val inactiveTabsFilter: (TabSessionState) -> Boolean = filter@{
if (!context.settings().inactiveTabsAreEnabled) {
return@filter false

@ -4,7 +4,6 @@
package org.mozilla.fenix.tabstray.browser
import android.content.Context
import androidx.recyclerview.widget.ConcatAdapter
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.tabstray.Tab
@ -13,23 +12,25 @@ import mozilla.components.concept.tabstray.TabsTray
import mozilla.components.feature.tabs.tabstray.TabsFeature
import mozilla.components.support.base.observer.Observable
import mozilla.components.support.base.observer.ObserverRegistry
import org.mozilla.fenix.ext.settings
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.utils.Settings
import kotlin.math.max
/**
* An intermediary layer to consume tabs from [TabsFeature] for sorting into the various adapters.
*/
class TabSorter(
private val context: Context,
private val settings: Settings,
private val concatAdapter: ConcatAdapter,
private val store: BrowserStore
) : TabsTray, Observable<TabsTray.Observer> by ObserverRegistry() {
private val groupsSet = mutableSetOf<String>()
override fun updateTabs(tabs: Tabs) {
val inactiveTabs = tabs.list.getInactiveTabs(context)
val searchTermTabs = tabs.list.getSearchGroupTabs(context)
val inactiveTabs = tabs.list.getInactiveTabs(settings)
val searchTermTabs = tabs.list.getSearchGroupTabs(settings)
val normalTabs = tabs.list - inactiveTabs - searchTermTabs
val selectedTabId = store.state.selectedTabId
@ -40,7 +41,11 @@ class TabSorter(
// Tab groups
// We don't need to provide a selectedId, because the [TabGroupAdapter] has that built-in with support from
// NormalBrowserPageViewHolder.scrollToTab.
val (groups, remainderTabs) = searchTermTabs.toSearchGroups()
val (groups, remainderTabs) = searchTermTabs.toSearchGroups(groupsSet)
groupsSet.clear()
groupsSet.addAll(groups.map { it.title })
concatAdapter.tabGroupAdapter.submitList(groups)
// Normal tabs.
@ -60,8 +65,8 @@ private fun List<Tab>.findSelectedIndex(tabId: String?): Int {
/**
* Returns a list of inactive tabs based on our preferences.
*/
private fun List<Tab>.getInactiveTabs(context: Context): List<Tab> {
val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled
private fun List<Tab>.getInactiveTabs(settings: Settings): List<Tab> {
val inactiveTabsEnabled = settings.inactiveTabsAreEnabled
return if (inactiveTabsEnabled) {
filter { !it.isActive(maxActiveTime) }
} else {
@ -72,9 +77,9 @@ private fun List<Tab>.getInactiveTabs(context: Context): List<Tab> {
/**
* Returns a list of search term tabs based on our preferences.
*/
private fun List<Tab>.getSearchGroupTabs(context: Context): List<Tab> {
val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled
val tabGroupsEnabled = context.settings().searchTermTabGroupsAreEnabled
private fun List<Tab>.getSearchGroupTabs(settings: Settings): List<Tab> {
val inactiveTabsEnabled = settings.inactiveTabsAreEnabled
val tabGroupsEnabled = settings.searchTermTabGroupsAreEnabled
return when {
tabGroupsEnabled && inactiveTabsEnabled ->
filter { it.searchTerm.isNotBlank() && it.isActive(maxActiveTime) }
@ -107,7 +112,7 @@ private fun Tab.isActive(maxActiveTime: Long): Boolean {
*
* See also: https://github.com/mozilla-mobile/android-components/issues/11012
*/
private fun List<Tab>.toSearchGroups(): Pair<List<TabGroupAdapter.Group>, List<Tab>> {
private fun List<Tab>.toSearchGroups(groupSet: Set<String>): Pair<List<TabGroupAdapter.Group>, List<Tab>> {
val data = groupBy { it.searchTerm.lowercase() }
val groupings = data.map { mapEntry ->
@ -127,7 +132,7 @@ private fun List<Tab>.toSearchGroups(): Pair<List<TabGroupAdapter.Group>, List<T
)
}
val groups = groupings.filter { it.tabs.size > 1 }.sortedBy { it.lastAccess }
val groups = groupings.filter { it.tabs.size > 1 || groupSet.contains(it.title) }.sortedBy { it.lastAccess }
val remainderTabs = (groupings - groups).flatMap { it.tabs }
return groups to remainderTabs

@ -11,8 +11,12 @@ import java.util.UUID
* Helper for writing tests that need a [Tab].
*/
fun createTab(
tabId: String = UUID.randomUUID().toString()
tabId: String = UUID.randomUUID().toString(),
lastAccess: Long = 0L,
searchTerm: String = ""
) = Tab(
tabId,
"https://mozilla.org"
id = tabId,
url = "https://mozilla.org",
lastAccess = lastAccess,
searchTerm = searchTerm
)

@ -0,0 +1,315 @@
/* 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 androidx.recyclerview.widget.ConcatAdapter
import io.mockk.every
import io.mockk.mockk
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.tabstray.Tabs
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
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
import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.TAB_GROUP_FEATURE_NAME
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
@RunWith(FenixRobolectricTestRunner::class)
class TabSorterTest {
private val context = testContext
private val settings: Settings = mockk()
private var inactiveTimestamp = 0L
@Before
fun setUp() {
every { settings.inactiveTabsAreEnabled }.answers { true }
every { settings.searchTermTabGroupsAreEnabled }.answers { true }
}
@Test
fun `WHEN updated with one normal tab THEN adapter have only one normal tab and no header`() {
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis())
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 1)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 0)
assertEquals(adapter.titleHeaderAdapter.itemCount, 0)
assertEquals(adapter.browserAdapter.itemCount, 1)
}
@Test
fun `WHEN updated with one normal tab and two search term tab THEN adapter have normal tab and a search group`() {
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis()),
createTab("tab2", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab3", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 2)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 1)
assertEquals(adapter.browserAdapter.itemCount, 1)
}
@Test
fun `WHEN updated with one normal tab, one inactive tab and two search term tab THEN adapter have normal tab, inactive tab and a search group`() {
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis()),
createTab("tab2", inactiveTimestamp),
createTab("tab3", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab4", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 3)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 1)
assertEquals(adapter.tabGroupAdapter.itemCount, 1)
assertEquals(adapter.browserAdapter.itemCount, 1)
}
@Test
fun `WHEN inactive tabs is off THEN adapter have no inactive tab`() {
every { settings.inactiveTabsAreEnabled }.answers { false }
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis()),
createTab("tab2", inactiveTimestamp),
createTab("tab3", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab4", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 3)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 1)
assertEquals(adapter.browserAdapter.itemCount, 2)
}
@Test
fun `WHEN search term tabs is off THEN adapter have no search term group`() {
every { settings.searchTermTabGroupsAreEnabled }.answers { false }
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis()),
createTab("tab2", inactiveTimestamp),
createTab("tab3", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab4", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 4)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 1)
assertEquals(adapter.tabGroupAdapter.itemCount, 0)
assertEquals(adapter.browserAdapter.itemCount, 3)
}
@Test
fun `WHEN both inactive tabs and search term tabs are off THEN adapter have only normal tabs`() {
every { settings.inactiveTabsAreEnabled }.answers { false }
every { settings.searchTermTabGroupsAreEnabled }.answers { false }
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis()),
createTab("tab2", inactiveTimestamp),
createTab("tab3", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab4", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 4)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 0)
assertEquals(adapter.browserAdapter.itemCount, 4)
}
@Test
fun `WHEN only one search term tab THEN there is no search group`() {
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 1)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 0)
assertEquals(adapter.browserAdapter.itemCount, 1)
}
@Test
fun `WHEN remove second last one search term tab THEN search group is kept even if there's only one tab`() {
val store = BrowserStore(
BrowserState(
tabs = emptyList()
)
)
val adapter = ConcatAdapter(
InactiveTabsAdapter(context, mock(), mock(), INACTIVE_TABS_FEATURE_NAME, settings),
TabGroupAdapter(context, mock(), mock(), TAB_GROUP_FEATURE_NAME),
TitleHeaderAdapter(store, context.settings()),
BrowserTabsAdapter(context, mock(), mock(), TABS_TRAY_FEATURE_NAME)
)
val tabSorter = TabSorter(settings, adapter, store)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis(), searchTerm = "mozilla"),
createTab("tab2", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 1)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 1)
assertEquals(adapter.browserAdapter.itemCount, 0)
tabSorter.updateTabs(
Tabs(
list = listOf(
createTab("tab1", System.currentTimeMillis(), searchTerm = "mozilla")
),
selectedIndex = 0
)
)
assertEquals(adapter.itemCount, 1)
assertEquals(adapter.inactiveTabsAdapter.inActiveTabsCount, 0)
assertEquals(adapter.tabGroupAdapter.itemCount, 1)
assertEquals(adapter.browserAdapter.itemCount, 0)
}
}
Loading…
Cancel
Save