Issue mozilla-mobile#21686: Move submitList calls into TabsAdapter

Co-authored-by: Roger Yang <royang@mozilla.com>
fork-maintainers · Oct 12, 2021 · aa1bd6b · aa1bd6b
1 parent 7e0146d
commit aa1bd6b
Show file tree

Hide file tree

Showing 5 changed files with 2 additions and 33 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt
@@ -105,10 +105,6 @@ class InactiveTabsAdapter(
     }
 
     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 object DiffCallback : DiffUtil.ItemCallback<Item>() {
         override fun areItemsTheSame(oldItem: Item, newItem: Item): Boolean {

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroupAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabGroupAdapter.kt
@@ -109,10 +109,6 @@ class TabGroupAdapter(
      * Not implemented; handled by nested [RecyclerView].
      */
     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 object DiffCallback : DiffUtil.ItemCallback<Group>() {
         override fun areItemsTheSame(oldItem: Group, newItem: Group) = oldItem.title == newItem.title

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
@@ -46,19 +46,10 @@ class TabSorter(
         // 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 {

diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsAdapter.kt
@@ -37,6 +37,8 @@ abstract class TabsAdapter<T : TabViewHolder>(
     override fun updateTabs(tabs: Tabs) {
         this.tabs = tabs
 
+        submitList(tabs.list)
+
         notifyObservers { onTabsUpdated() }
     }
 
@@ -47,23 +49,9 @@ abstract class TabsAdapter<T : TabViewHolder>(
         holder.bind(tabs.list[position], isTabSelected(tabs, position), styling, this)
     }
 
-    override fun getItemCount(): Int = tabs?.list?.size ?: 0
-
     final override fun isTabSelected(tabs: Tabs, position: Int): Boolean =
         tabs.selectedIndex == position
 
-    final override fun onTabsChanged(position: Int, count: Int) =
-        notifyItemRangeChanged(position, count)
-
-    final override fun onTabsInserted(position: Int, count: Int) =
-        notifyItemRangeInserted(position, count)
-
-    final override fun onTabsMoved(fromPosition: Int, toPosition: Int) =
-        notifyItemMoved(fromPosition, toPosition)
-
-    final override fun onTabsRemoved(position: Int, count: Int) =
-        notifyItemRangeRemoved(position, count)
-
     private object DiffCallback : DiffUtil.ItemCallback<Tab>() {
         override fun areItemsTheSame(oldItem: Tab, newItem: Tab): Boolean {
             return oldItem.id == newItem.id

diff --git a/...src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt b/...src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt
@@ -51,7 +51,6 @@ class AbstractBrowserPageViewHolderTest {
                 selectedIndex = 0
             )
         )
-        adapter.onTabsInserted(0, 1)
 
         assertTrue(trayList.visibility == VISIBLE)
         assertTrue(emptyList.visibility == GONE)
@@ -74,7 +73,6 @@ class AbstractBrowserPageViewHolderTest {
                 selectedIndex = 0
             )
         )
-        adapter.onTabsInserted(0, 0)
 
         assertTrue(trayList.visibility == GONE)
         assertTrue(emptyList.visibility == VISIBLE)