From 58657ebdefd259ee2bfea27b047cf5f077496498 Mon Sep 17 00:00:00 2001 From: "codrut.topliceanu" Date: Mon, 14 Jun 2021 15:13:48 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/19131 - Fixes top sites refresh and interactability --- .../viewholders/TopSitePagerViewHolder.kt | 6 ++-- .../topsites/TopSiteItemViewHolder.kt | 6 ---- .../viewholders/topsites/TopSitesAdapter.kt | 10 +----- .../topsites/TopSitesPagerAdapter.kt | 31 +++++++++++++------ .../topsites/TopSitesAdapterTest.kt | 2 +- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TopSitePagerViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TopSitePagerViewHolder.kt index 471a0d796c..72e44709e0 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TopSitePagerViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TopSitePagerViewHolder.kt @@ -53,9 +53,9 @@ class TopSitePagerViewHolder( } fun update(payload: AdapterItem.TopSitePagerPayload) { - for (item in payload.changed) { - topSitesPagerAdapter.notifyItemChanged(currentPage, payload) - } + // Due to offscreenPageLimit = 1 we need to update both pages manually here + topSitesPagerAdapter.notifyItemChanged(0, payload) + topSitesPagerAdapter.notifyItemChanged(1, payload) } fun bind(topSites: List) { diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSiteItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSiteItemViewHolder.kt index 7ca0da2ca5..e337a2b4b6 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSiteItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSiteItemViewHolder.kt @@ -11,7 +11,6 @@ import android.view.View import android.widget.PopupWindow import androidx.appcompat.content.res.AppCompatResources.getDrawable import kotlinx.android.synthetic.main.top_site_item.* -import kotlinx.android.synthetic.main.top_site_item.view.* import mozilla.components.browser.menu.BrowserMenuBuilder import mozilla.components.browser.menu.item.SimpleBrowserMenuItem import mozilla.components.feature.top.sites.TopSite @@ -93,11 +92,6 @@ class TopSiteItemViewHolder( this.topSite = topSite } - fun bind(topSitePayload: TopSitesAdapter.TopSitePayload) { - itemView.top_site_title.text = topSitePayload.newTitle - topSite = topSite.copy(title = topSitePayload.newTitle) - } - private fun onTouchEvent( v: View, event: MotionEvent, diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapter.kt index 5519086992..587b6a86fc 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapter.kt @@ -8,7 +8,6 @@ import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter -import kotlinx.android.synthetic.main.top_site_item.view.* import mozilla.components.feature.top.sites.TopSite import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor import org.mozilla.fenix.perf.StartupTimeline @@ -39,17 +38,10 @@ class TopSitesAdapter( is TopSite -> { holder.bind((payloads[0] as TopSite)) } - is TopSitePayload -> { - holder.bind(payloads[0] as TopSitePayload) - } } } } - data class TopSitePayload( - val newTitle: String? - ) - internal object TopSitesDiffCallback : DiffUtil.ItemCallback() { override fun areItemsTheSame(oldItem: TopSite, newItem: TopSite) = oldItem.id == newItem.id @@ -58,7 +50,7 @@ class TopSitesAdapter( override fun getChangePayload(oldItem: TopSite, newItem: TopSite): Any? { return if (oldItem.id == newItem.id && oldItem.url == newItem.url && oldItem.title != newItem.title) { - TopSitePayload(newItem.title) + newItem } else null } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesPagerAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesPagerAdapter.kt index e60cc4cd65..5852fa7627 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesPagerAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesPagerAdapter.kt @@ -10,7 +10,7 @@ import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter import kotlinx.android.synthetic.main.component_top_sites.view.* import mozilla.components.feature.top.sites.TopSite -import org.mozilla.fenix.home.sessioncontrol.AdapterItem +import org.mozilla.fenix.home.sessioncontrol.AdapterItem.TopSitePagerPayload import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor import org.mozilla.fenix.home.sessioncontrol.viewholders.TopSitePagerViewHolder.Companion.TOP_SITES_PER_PAGE import org.mozilla.fenix.home.sessioncontrol.viewholders.TopSiteViewHolder @@ -33,15 +33,26 @@ class TopSitesPagerAdapter( if (payloads.isNullOrEmpty()) { onBindViewHolder(holder, position) } else { - if (payloads[0] is AdapterItem.TopSitePagerPayload) { + if (payloads[0] is TopSitePagerPayload) { val adapter = holder.itemView.top_sites_list.adapter as TopSitesAdapter - val payload = payloads[0] as AdapterItem.TopSitePagerPayload - for (item in payload.changed) { - adapter.notifyItemChanged( - item.first % TOP_SITES_PER_PAGE, - TopSitesAdapter.TopSitePayload(item.second.title) - ) - } + val payload = payloads[0] as TopSitePagerPayload + + update(payload, position, adapter) + } + } + } + + private fun update( + payload: TopSitePagerPayload, + position: Int, + adapter: TopSitesAdapter + ) { + // Only currently selected page items need to be updated. + for (item in payload.changed) { + if (item.first < TOP_SITES_PER_PAGE && position == 0) { + adapter.notifyItemChanged(item.first, item.second) + } else if (item.first >= TOP_SITES_PER_PAGE && position == 1) { + adapter.notifyItemChanged(item.first - TOP_SITES_PER_PAGE, item.second) } } } @@ -67,7 +78,7 @@ class TopSitesPagerAdapter( changed.add(Pair(index, item)) } } - return if (changed.isNotEmpty()) AdapterItem.TopSitePagerPayload(changed) else null + return if (changed.isNotEmpty()) TopSitePagerPayload(changed) else null } } } diff --git a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapterTest.kt index b64a0130de..91533fe91a 100644 --- a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/topsites/TopSitesAdapterTest.kt @@ -29,7 +29,7 @@ class TopSitesAdapterTest { assertEquals( TopSitesAdapter.TopSitesDiffCallback.getChangePayload(topSite, topSite2), - TopSitesAdapter.TopSitePayload("Title2") + topSite.copy(title = "Title2") ) val topSite3 = TopSite(