[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21641: Do not add a group of only one tab

pull/600/head
Jonathan Almeida 3 years ago committed by Jonathan Almeida
parent ad1cff2300
commit 0c5285cf05

@ -9,18 +9,15 @@ import android.util.AttributeSet
import androidx.recyclerview.widget.ConcatAdapter
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.tabstray.TabViewHolder
import mozilla.components.concept.tabstray.Tab
import mozilla.components.concept.tabstray.TabsTray
import mozilla.components.feature.tabs.tabstray.TabsFeature
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.tabstray.TrayPagerAdapter.Companion.TABS_TRAY_FEATURE_NAME
import org.mozilla.fenix.tabstray.ext.browserAdapter
import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter
import org.mozilla.fenix.tabstray.ext.isNormalTabActive
import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm
import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithoutSearchTerm
import org.mozilla.fenix.tabstray.ext.isNormalTabWithoutSearchTerm
import org.mozilla.fenix.tabstray.ext.isNormalTabWithSearchTerm
import org.mozilla.fenix.tabstray.ext.isNormalTabInactive
import org.mozilla.fenix.tabstray.ext.tabGroupAdapter
import java.util.concurrent.TimeUnit
/**
@ -39,85 +36,33 @@ class NormalBrowserTrayList @JvmOverloads constructor(
defStyleAttr: Int = 0
) : AbstractBrowserTrayList(context, attrs, defStyleAttr) {
private val swipeDelegate = SwipeToDeleteDelegate()
private val concatAdapter by lazy { adapter as ConcatAdapter }
override val tabsFeature by lazy {
val tabsAdapter = concatAdapter.browserAdapter
val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled
val searchTermTabGroupsAreEnabled = context.settings().searchTermTabGroupsAreEnabled
val tabFilter: (TabSessionState) -> Boolean = {
when {
searchTermTabGroupsAreEnabled && inactiveTabsEnabled ->
it.isNormalTabActiveWithoutSearchTerm(maxActiveTime)
inactiveTabsEnabled -> it.isNormalTabActive(maxActiveTime)
searchTermTabGroupsAreEnabled -> it.isNormalTabWithoutSearchTerm()
else -> !it.content.private
}
}
TabsFeature(
tabsAdapter,
context.components.core.store,
selectTabUseCase,
removeTabUseCase,
tabFilter,
{}
)
}
private val searchTermFeature by lazy {
val store = context.components.core.store
val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled
val searchTermTabGroupsAreEnabled = context.settings().searchTermTabGroupsAreEnabled
val tabFilter: (TabSessionState) -> Boolean = {
when {
searchTermTabGroupsAreEnabled && inactiveTabsEnabled ->
it.isNormalTabActiveWithSearchTerm(maxActiveTime)
searchTermTabGroupsAreEnabled -> it.isNormalTabWithSearchTerm()
else -> false
}
}
val tabsAdapter = concatAdapter.tabGroupAdapter
TabsFeature(
tabsAdapter,
store,
selectTabUseCase,
removeTabUseCase,
tabFilter,
{}
)
}
/**
* NB: The setup for this feature is a bit complicated without a better dependency injection
* solution to scope it down to just this view.
*/
private val inactiveFeature by lazy {
val store = context.components.core.store
private val tabSorter by lazy { TabSorter(context, concatAdapter, context.components.core.store) }
private val inactiveTabsInteractor by lazy {
val tabFilter: (TabSessionState) -> Boolean = filter@{
if (!context.settings().inactiveTabsAreEnabled) {
return@filter false
}
it.isNormalTabInactive(maxActiveTime)
}
val tabsAdapter = concatAdapter.inactiveTabsAdapter.apply {
inactiveTabsInteractor = DefaultInactiveTabsInteractor(
InactiveTabsController(store, tabFilter, this, context.components.analytics.metrics)
DefaultInactiveTabsInteractor(
InactiveTabsController(
context.components.core.store,
tabFilter,
concatAdapter.inactiveTabsAdapter,
context.components.analytics.metrics
)
}
)
}
override val tabsFeature by lazy {
TabsFeature(
tabsAdapter,
store,
tabSorter,
context.components.core.store,
selectTabUseCase,
removeTabUseCase,
tabFilter,
{ !it.content.private },
{}
)
}
@ -135,10 +80,12 @@ class NormalBrowserTrayList @JvmOverloads constructor(
override fun onAttachedToWindow() {
super.onAttachedToWindow()
inactiveFeature.start()
searchTermFeature.start()
concatAdapter.inactiveTabsAdapter.inactiveTabsInteractor = inactiveTabsInteractor
tabsFeature.start()
concatAdapter.browserAdapter.register(swipeDelegate)
touchHelper.attachToRecyclerView(this)
}
@ -146,9 +93,22 @@ class NormalBrowserTrayList @JvmOverloads constructor(
super.onDetachedFromWindow()
tabsFeature.stop()
searchTermFeature.stop()
inactiveFeature.stop()
concatAdapter.browserAdapter.unregister(swipeDelegate)
touchHelper.attachToRecyclerView(null)
}
/**
* A delegate for handling open/selected events from swipe-to-delete gestures.
*/
inner class SwipeToDeleteDelegate : TabsTray.Observer {
override fun onTabClosed(tab: Tab) {
removeTabUseCase.invoke(tab.id, TABS_TRAY_FEATURE_NAME)
}
override fun onTabSelected(tab: Tab) {
selectTabUseCase.invoke(tab.id)
}
}
}

@ -20,7 +20,6 @@ import org.mozilla.fenix.components.Components
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.selection.SelectionHolder
import org.mozilla.fenix.tabstray.TabsTrayStore
import kotlin.math.max
import mozilla.components.concept.tabstray.Tab as TabsTrayTab
import mozilla.components.support.base.observer.Observable
@ -102,27 +101,9 @@ class TabGroupAdapter(
}
/**
* Creates a grouping of data classes for how groupings will be structured.
* Not implemented; implementation is handled [List<Tab>.toSearchGroups]
*/
override fun updateTabs(tabs: Tabs) {
val data = tabs.list.groupBy { it.searchTerm.lowercase() }
val grouping = data.map { mapEntry ->
val searchTerm = mapEntry.key.replaceFirstChar(Char::uppercase)
val groupTabs = mapEntry.value
val groupMax = groupTabs.fold(0L) { acc, tab ->
max(tab.lastAccess, acc)
}
Group(
title = searchTerm,
tabs = groupTabs,
lastAccess = groupMax
)
}.sortedBy { it.lastAccess }
submitList(grouping)
}
override fun updateTabs(tabs: Tabs) = throw UnsupportedOperationException("Use submitList instead.")
/**
* Not implemented; handled by nested [RecyclerView].

@ -0,0 +1,143 @@
/* 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 android.content.Context
import androidx.recyclerview.widget.ConcatAdapter
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.tabstray.Tab
import mozilla.components.concept.tabstray.Tabs
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 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 concatAdapter: ConcatAdapter,
private val store: BrowserStore
) : TabsTray, Observable<TabsTray.Observer> by ObserverRegistry() {
override fun updateTabs(tabs: Tabs) {
val inactiveTabs = tabs.list.getInactiveTabs(context)
val searchTermTabs = tabs.list.getSearchGroupTabs(context)
val normalTabs = tabs.list - inactiveTabs - searchTermTabs
val selectedTabId = store.state.selectedTabId
// Inactive tabs
val selectedInactiveIndex = inactiveTabs.findSelectedIndex(selectedTabId)
concatAdapter.inactiveTabsAdapter.updateTabs((Tabs(inactiveTabs, selectedInactiveIndex)))
// 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()
concatAdapter.tabGroupAdapter.submitList(groups)
// Normal tabs.
val totalNormalTabs = (normalTabs + remainderTabs)
val selectedTabIndex = totalNormalTabs.findSelectedIndex(selectedTabId)
// N.B: For regular tabs, we cannot use submitList alone, because the `TabsAdapter` needs to have a reference
// to the new tabs in it. We considered moving the call within `updateTabs` but this would have the side-effect
// of notifying the adapter twice for private tabs which shared the `TabsAdapter`.
concatAdapter.browserAdapter.updateTabs(Tabs(totalNormalTabs, selectedTabIndex))
concatAdapter.browserAdapter.submitList(totalNormalTabs)
}
override fun isTabSelected(tabs: Tabs, position: Int): Boolean = false
override fun onTabsChanged(position: Int, count: Int) = Unit
override fun onTabsInserted(position: Int, count: Int) = Unit
override fun onTabsMoved(fromPosition: Int, toPosition: Int) = Unit
override fun onTabsRemoved(position: Int, count: Int) = Unit
}
private fun List<Tab>.findSelectedIndex(tabId: String?): Int {
val id = tabId ?: return -1
return indexOfFirst { it.id == id }
}
/**
* Returns a list of inactive tabs based on our preferences.
*/
private fun List<Tab>.getInactiveTabs(context: Context): List<Tab> {
val inactiveTabsEnabled = context.settings().inactiveTabsAreEnabled
return if (inactiveTabsEnabled) {
filter { !it.isActive(maxActiveTime) }
} else {
emptyList()
}
}
/**
* 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
return when {
tabGroupsEnabled && inactiveTabsEnabled ->
filter { it.searchTerm.isNotBlank() && it.isActive(maxActiveTime) }
tabGroupsEnabled ->
filter { it.searchTerm.isNotBlank() }
else -> emptyList()
}
}
/**
* Returns true if a tab has not been selected since [maxActiveTime].
*
* N.B: This is duplicated from [TabSessionState.isActive(Long)] to work for [Tab].
*
* See also: https://github.com/mozilla-mobile/android-components/issues/11012
*/
private fun Tab.isActive(maxActiveTime: Long): Boolean {
val lastActiveTime = maxOf(lastAccess, createdAt)
val now = System.currentTimeMillis()
return (now - lastActiveTime <= maxActiveTime)
}
/**
* Creates a list of grouped search term tabs sorted by last access time and a list of tabs
* that have search terms but would only create groups with a single tab.
*
* N.B: This is duplicated from [List<TabSessionState>.toSearchGroup()] to work for [Tab].
*
* See also: https://github.com/mozilla-mobile/android-components/issues/11012
*/
private fun List<Tab>.toSearchGroups(): Pair<List<TabGroupAdapter.Group>, List<Tab>> {
val data = groupBy { it.searchTerm.lowercase() }
val groupings = data.map { mapEntry ->
// Uppercase since we use it for the title.
val searchTerm = mapEntry.key.replaceFirstChar(Char::uppercase)
val groupTabs = mapEntry.value
// Calculate when the group was last used.
val groupMax = groupTabs.fold(0L) { acc, tab ->
max(tab.lastAccess, acc)
}
TabGroupAdapter.Group(
title = searchTerm,
tabs = groupTabs,
lastAccess = groupMax
)
}
val groups = groupings.filter { it.tabs.size > 1 }.sortedBy { it.lastAccess }
val remainderTabs = (groupings - groups).flatMap { it.tabs }
return groups to remainderTabs
}
Loading…
Cancel
Save