Close #22061: Let TabSorter decide if title header should be shown in tabs tray

upstream-sync
Roger Yang 3 years ago committed by mergify[bot]
parent 2d8492d33f
commit 70fc6fc20f

@ -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)
)
}

@ -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

@ -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<TitleHeaderAdapter.Header, TitleHeaderAdapter.HeaderViewHolder>(DiffCallback) {
class TitleHeaderAdapter :
ListAdapter<TitleHeaderAdapter.Header, TitleHeaderAdapter.HeaderViewHolder>(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 {

@ -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<BrowserState>(store) {
override suspend fun onState(flow: Flow<BrowserState>) {
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)
}
}
}
}

@ -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<TabGroup> = if (searchTermTabGroupsAreEnabled) {
normalTabs.toSearchGroup().first
} else {
emptyList()
}

@ -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)
}
}

@ -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)
}
}
Loading…
Cancel
Save