2
0
mirror of https://github.com/fork-maintainers/iceraven-browser synced 2024-11-03 23:15:31 +00:00

[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21236: Fixes empty tray visibility logic

This is a bug we noticed after landing search term grouping.

An adapter can submit an empty list of items to the `ConcatAdapter`
early. This has the side-effect of triggering our `observeFirstInsert`
too soon and therefore updating the visibility to show the empty tray
placeholder and never switches back.

Our solution is to keep a constant observer on the adapter so we can
perform the visibility check on every insert/remove.

Co-authored-by: Roger Yang <royang@mozilla.com>
This commit is contained in:
Jonathan Almeida 2021-09-17 12:38:45 -04:00 committed by Jonathan Almeida
parent b239d7d934
commit c2d385f142
5 changed files with 73 additions and 4 deletions

View File

@ -106,6 +106,14 @@ class TrayPagerAdapter(
}
}
override fun onViewAttachedToWindow(holder: AbstractPageViewHolder) {
holder.attachedToWindow()
}
override fun onViewDetachedFromWindow(holder: AbstractPageViewHolder) {
holder.detachedFromWindow()
}
override fun getItemCount(): Int = TRAY_TABS_COUNT
companion object {

View File

@ -13,8 +13,8 @@ import androidx.recyclerview.widget.RecyclerView
import org.mozilla.fenix.R
import org.mozilla.fenix.tabstray.TabsTrayInteractor
import org.mozilla.fenix.tabstray.TabsTrayStore
import org.mozilla.fenix.tabstray.TrayPagerAdapter
import org.mozilla.fenix.tabstray.browser.AbstractBrowserTrayList
import org.mozilla.fenix.tabstray.ext.observeFirstInsert
/**
* A shared view holder for browser tabs tray list.
@ -27,6 +27,9 @@ abstract class AbstractBrowserPageViewHolder(
private val trayList: AbstractBrowserTrayList = itemView.findViewById(R.id.tray_list_item)
private val emptyList: TextView = itemView.findViewById(R.id.tab_tray_empty_view)
private var adapterObserver: RecyclerView.AdapterDataObserver? = null
private var adapterRef: RecyclerView.Adapter<out RecyclerView.ViewHolder>? = null
abstract val emptyStringText: String
init {
@ -48,14 +51,53 @@ abstract class AbstractBrowserPageViewHolder(
adapter: RecyclerView.Adapter<out RecyclerView.ViewHolder>,
layoutManager: RecyclerView.LayoutManager
) {
adapter.observeFirstInsert {
updateTrayVisibility(adapter.itemCount)
}
adapterRef = adapter
scrollToTab(adapter, layoutManager)
trayList.layoutManager = layoutManager
trayList.adapter = adapter
}
/**
* When the [RecyclerView.Adapter] is attached to the window we register a data observer to
* always check whether to call [updateTrayVisibility].
*
* We keep a constant observer instead of using [RecyclerView.Adapter.observeFirstInsert],
* because some adapters can insert empty lists and trigger the one-shot observer too soon.
*
* See also [AbstractPageViewHolder.attachedToWindow].
*/
override fun attachedToWindow() {
adapterRef?.let { adapter ->
adapterObserver = object : RecyclerView.AdapterDataObserver() {
override fun onItemRangeInserted(positionStart: Int, itemCount: Int) {
updateTrayVisibility(adapter.itemCount)
}
override fun onItemRangeRemoved(positionstart: Int, itemcount: Int) {
updateTrayVisibility(adapter.itemCount)
}
}
adapterObserver?.let {
adapter.registerAdapterDataObserver(it)
}
}
}
/**
* [RecyclerView.AdapterDataObserver]s are responsible to be unregistered when they are done,
* so we do that here when our [TrayPagerAdapter] page is detached from the window.
*
* See also [AbstractPageViewHolder.detachedFromWindow].
*/
override fun detachedFromWindow() {
adapterObserver?.let {
adapterRef?.unregisterAdapterDataObserver(it)
adapterObserver = null
}
}
private fun updateTrayVisibility(size: Int) {
if (size == 0) {
trayList.visibility = GONE

View File

@ -15,7 +15,21 @@ abstract class AbstractPageViewHolder constructor(
val containerView: View
) : RecyclerView.ViewHolder(containerView) {
/**
* Invoked when the nested [RecyclerView.Adapter] is bound to the [RecyclerView.ViewHolder].
*/
abstract fun bind(
adapter: RecyclerView.Adapter<out RecyclerView.ViewHolder>
)
/**
* Invoked when the [RecyclerView.ViewHolder] is attached from the window. This could have
* previously been bound and is now attached again.
*/
abstract fun attachedToWindow()
/**
* Invoked when the [RecyclerView.ViewHolder] is detached from the window.
*/
abstract fun detachedFromWindow()
}

View File

@ -27,6 +27,9 @@ class SyncedTabsPageViewHolder(
binding.syncedTabsTrayLayout.tabsTrayStore = tabsTrayStore
}
override fun detachedFromWindow() = Unit // no-op
override fun attachedToWindow() = Unit // no-op
companion object {
const val LAYOUT_ID = R.layout.component_sync_tabs_tray_layout
}

View File

@ -41,6 +41,7 @@ class AbstractBrowserPageViewHolderTest {
val emptyList: TextView = itemView.findViewById(R.id.tab_tray_empty_view)
viewHolder.bind(adapter)
viewHolder.attachedToWindow()
adapter.updateTabs(
Tabs(
@ -65,6 +66,7 @@ class AbstractBrowserPageViewHolderTest {
val emptyList: TextView = itemView.findViewById(R.id.tab_tray_empty_view)
viewHolder.bind(adapter)
viewHolder.attachedToWindow()
adapter.updateTabs(
Tabs(