Keep the tabs adapter over the life of the viewpager (#3255)

Make `tabs` `var` instead of `val` in `MainPagerAdapter` so it can be updated
when tabs change.

Then detach the `tabLayoutMediator`, update the tabs, and call
`notifyItemRangeChanged` in `setupTabs()`.

This fixes a bug (not sure if it's this code, or in ViewPager2) where
assigning a new adapter to the view pager seemed to result in a leak of one
or more fragments. This wasn't user-visible, but it's a leak, and it becomes
user-visible when fragments want to display menus.

This also fixes two other bugs:

1. Be on the left-most tab. Scroll down a bit. Then modify the tabs at
   "Account preferences > tabs", but keep the left-most tab as-is.

   Then go back to MainActivity. Your reading position in the left-most
   tab has been jumped to the top.

2. Be on any non-left-most tab. Then modify the tab list by reordering tabs
   (adding/removing tabs is also OK).

   Then go back to MainActivity. Your tab selection has been overridden,
   and the left-most tab has been selected.

Because the fragments are not destroyed unnecessarily your reading position
is retained. And it remembers the tab you had selected, and as long as that
tab is still present you will be returned to it, even if it's changed
position in the list.

Fixes https://github.com/tuskyapp/Tusky/issues/3251
This commit is contained in:
Nik Clayton 2023-02-15 19:17:59 +01:00 committed by GitHub
parent 53d4a02bdc
commit 196ebdb386
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 25 deletions

View file

@ -34,6 +34,7 @@ import android.view.View
import android.widget.ImageView import android.widget.ImageView
import androidx.activity.OnBackPressedCallback import androidx.activity.OnBackPressedCallback
import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AlertDialog
import androidx.appcompat.content.res.AppCompatResources
import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.app.ActivityCompat import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
@ -170,6 +171,12 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
// We need to know if the emoji pack has been changed // We need to know if the emoji pack has been changed
private var selectedEmojiPack: String? = null private var selectedEmojiPack: String? = null
/** Mediate between binding.viewPager and the chosen tab layout */
private var tabLayoutMediator: TabLayoutMediator? = null
/** Adapter for the different timeline tabs */
private lateinit var tabAdapter: MainPagerAdapter
override fun onCreate(savedInstanceState: Bundle?) { override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState) super.onCreate(savedInstanceState)
@ -275,6 +282,13 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
fetchAnnouncements() fetchAnnouncements()
// Initialise the tab adapter and set to viewpager. Fragments appear to be leaked if the
// adapter changes over the life of the viewPager (the adapter, not its contents), so set
// the initial list of tabs to empty, and set the full list later in setupTabs(). See
// https://github.com/tuskyapp/Tusky/issues/3251 for details.
tabAdapter = MainPagerAdapter(emptyList(), this)
binding.viewPager.adapter = tabAdapter
setupTabs(showNotificationTab) setupTabs(showNotificationTab)
eventHub.events eventHub.events
@ -655,7 +669,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
} }
private fun setupTabs(selectNotificationTab: Boolean) { private fun setupTabs(selectNotificationTab: Boolean) {
val activeTabLayout = if (preferences.getString("mainNavPosition", "top") == "bottom") { val activeTabLayout = if (preferences.getString(PrefKeys.MAIN_NAV_POSITION, "top") == "bottom") {
val actionBarSize = getDimension(this, androidx.appcompat.R.attr.actionBarSize) val actionBarSize = getDimension(this, androidx.appcompat.R.attr.actionBarSize)
val fabMargin = resources.getDimensionPixelSize(R.dimen.fabMargin) val fabMargin = resources.getDimensionPixelSize(R.dimen.fabMargin)
(binding.composeButton.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = actionBarSize + fabMargin (binding.composeButton.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = actionBarSize + fabMargin
@ -668,29 +682,36 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
binding.tabLayout binding.tabLayout
} }
// Save the previous tab so it can be restored later
val previousTab = tabAdapter.tabs.getOrNull(binding.viewPager.currentItem)
val tabs = accountManager.activeAccount!!.tabPreferences val tabs = accountManager.activeAccount!!.tabPreferences
val adapter = MainPagerAdapter(tabs, this) // Detach any existing mediator before changing tab contents and attaching a new mediator
binding.viewPager.adapter = adapter tabLayoutMediator?.detach()
TabLayoutMediator(activeTabLayout, binding.viewPager) { _: TabLayout.Tab?, _: Int -> }.attach()
activeTabLayout.removeAllTabs()
for (i in tabs.indices) {
val tab = activeTabLayout.newTab()
.setIcon(tabs[i].icon)
if (tabs[i].id == LIST) {
tab.contentDescription = tabs[i].arguments[1]
} else {
tab.setContentDescription(tabs[i].text)
}
activeTabLayout.addTab(tab)
if (tabs[i].id == NOTIFICATIONS) { tabAdapter.tabs = tabs
notificationTabPosition = i tabAdapter.notifyItemRangeChanged(0, tabs.size)
if (selectNotificationTab) {
tab.select() tabLayoutMediator = TabLayoutMediator(activeTabLayout, binding.viewPager, true) {
} tab: TabLayout.Tab, position: Int ->
} tab.icon = AppCompatResources.getDrawable(this@MainActivity, tabs[position].icon)
tab.contentDescription = when (tabs[position].id) {
LIST -> tabs[position].arguments[position]
else -> getString(tabs[position].text)
} }
}.also { it.attach() }
// Selected tab is either
// - Notification tab (if appropriate)
// - The previously selected tab (if it hasn't been removed)
// - Left-most tab
val position = if (selectNotificationTab) {
tabs.indexOfFirst { it.id == NOTIFICATIONS }
} else {
previousTab?.let { tabs.indexOfFirst { it == previousTab } }
}.takeIf { it != -1 } ?: 0
binding.viewPager.setCurrentItem(position, false)
val pageMargin = resources.getDimensionPixelSize(R.dimen.tab_page_margin) val pageMargin = resources.getDimensionPixelSize(R.dimen.tab_page_margin)
binding.viewPager.setPageTransformer(MarginPageTransformer(pageMargin)) binding.viewPager.setPageTransformer(MarginPageTransformer(pageMargin))
@ -710,18 +731,18 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
binding.mainToolbar.title = tabs[tab.position].title(this@MainActivity) binding.mainToolbar.title = tabs[tab.position].title(this@MainActivity)
refreshComposeButtonState(adapter, tab.position) refreshComposeButtonState(tabAdapter, tab.position)
} }
override fun onTabUnselected(tab: TabLayout.Tab) {} override fun onTabUnselected(tab: TabLayout.Tab) {}
override fun onTabReselected(tab: TabLayout.Tab) { override fun onTabReselected(tab: TabLayout.Tab) {
val fragment = adapter.getFragment(tab.position) val fragment = tabAdapter.getFragment(tab.position)
if (fragment is ReselectableFragment) { if (fragment is ReselectableFragment) {
(fragment as ReselectableFragment).onReselect() (fragment as ReselectableFragment).onReselect()
} }
refreshComposeButtonState(adapter, tab.position) refreshComposeButtonState(tabAdapter, tab.position)
} }
}.also { }.also {
activeTabLayout.addOnTabSelectedListener(it) activeTabLayout.addOnTabSelectedListener(it)
@ -730,7 +751,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
val activeTabPosition = if (selectNotificationTab) notificationTabPosition else 0 val activeTabPosition = if (selectNotificationTab) notificationTabPosition else 0
binding.mainToolbar.title = tabs[activeTabPosition].title(this@MainActivity) binding.mainToolbar.title = tabs[activeTabPosition].title(this@MainActivity)
binding.mainToolbar.setOnClickListener { binding.mainToolbar.setOnClickListener {
(adapter.getFragment(activeTabLayout.selectedTabPosition) as? ReselectableFragment)?.onReselect() (tabAdapter.getFragment(activeTabLayout.selectedTabPosition) as? ReselectableFragment)?.onReselect()
} }
updateProfiles() updateProfiles()

View file

@ -20,7 +20,7 @@ import androidx.fragment.app.FragmentActivity
import com.keylesspalace.tusky.TabData import com.keylesspalace.tusky.TabData
import com.keylesspalace.tusky.util.CustomFragmentStateAdapter import com.keylesspalace.tusky.util.CustomFragmentStateAdapter
class MainPagerAdapter(val tabs: List<TabData>, activity: FragmentActivity) : CustomFragmentStateAdapter(activity) { class MainPagerAdapter(var tabs: List<TabData>, activity: FragmentActivity) : CustomFragmentStateAdapter(activity) {
override fun createFragment(position: Int): Fragment { override fun createFragment(position: Int): Fragment {
val tab = tabs[position] val tab = tabs[position]