From 74fd0432900a73b5e79247a2235df72e96c028f3 Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Thu, 7 Oct 2021 16:30:06 -0400 Subject: [PATCH] Issue #21686: Move submitList calls into TabsAdapter Co-authored-by: Roger Yang --- .../tabstray/browser/InactiveTabsAdapter.kt | 4 ---- .../fenix/tabstray/browser/TabGroupAdapter.kt | 4 ---- .../mozilla/fenix/tabstray/browser/TabSorter.kt | 9 --------- .../fenix/tabstray/browser/TabsAdapter.kt | 16 ++-------------- .../AbstractBrowserPageViewHolderTest.kt | 2 -- 5 files changed, 2 insertions(+), 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 index 49dec1eee..473950a7e 100644 --- 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() { 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 index 1298f85e5..17b000ada 100644 --- 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() { 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 index ddc8aba3e..de9e1232b 100644 --- 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.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 index 77aa6509c..6533009f7 100644 --- 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( override fun updateTabs(tabs: Tabs) { this.tabs = tabs + submitList(tabs.list) + notifyObservers { onTabsUpdated() } } @@ -47,23 +49,9 @@ abstract class TabsAdapter( 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() { override fun areItemsTheSame(oldItem: Tab, newItem: Tab): Boolean { return oldItem.id == newItem.id diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt index 7bbff9acc..fc39a0ebd 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolderTest.kt +++ b/app/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)