Skip to content

Commit

Permalink
Keep the tabs adapter over the life of the viewpager (#3255)
Browse files Browse the repository at this point in the history
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 #3251
  • Loading branch information
Nik Clayton committed Feb 15, 2023
1 parent 53d4a02 commit 196ebdb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 25 deletions.
69 changes: 45 additions & 24 deletions app/src/main/java/com/keylesspalace/tusky/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import android.view.View
import android.widget.ImageView
import androidx.activity.OnBackPressedCallback
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.content.res.AppCompatResources
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat
Expand Down Expand Up @@ -170,6 +171,12 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
// We need to know if the emoji pack has been changed
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?) {
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -275,6 +282,13 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje

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)

eventHub.events
Expand Down Expand Up @@ -655,7 +669,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
}

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 fabMargin = resources.getDimensionPixelSize(R.dimen.fabMargin)
(binding.composeButton.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = actionBarSize + fabMargin
Expand All @@ -668,29 +682,36 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
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 adapter = MainPagerAdapter(tabs, this)
binding.viewPager.adapter = adapter
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)
// Detach any existing mediator before changing tab contents and attaching a new mediator
tabLayoutMediator?.detach()

if (tabs[i].id == NOTIFICATIONS) {
notificationTabPosition = i
if (selectNotificationTab) {
tab.select()
}
tabAdapter.tabs = tabs
tabAdapter.notifyItemRangeChanged(0, tabs.size)

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)
binding.viewPager.setPageTransformer(MarginPageTransformer(pageMargin))
Expand All @@ -710,18 +731,18 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje

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 onTabReselected(tab: TabLayout.Tab) {
val fragment = adapter.getFragment(tab.position)
val fragment = tabAdapter.getFragment(tab.position)
if (fragment is ReselectableFragment) {
(fragment as ReselectableFragment).onReselect()
}

refreshComposeButtonState(adapter, tab.position)
refreshComposeButtonState(tabAdapter, tab.position)
}
}.also {
activeTabLayout.addOnTabSelectedListener(it)
Expand All @@ -730,7 +751,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
val activeTabPosition = if (selectNotificationTab) notificationTabPosition else 0
binding.mainToolbar.title = tabs[activeTabPosition].title(this@MainActivity)
binding.mainToolbar.setOnClickListener {
(adapter.getFragment(activeTabLayout.selectedTabPosition) as? ReselectableFragment)?.onReselect()
(tabAdapter.getFragment(activeTabLayout.selectedTabPosition) as? ReselectableFragment)?.onReselect()
}

updateProfiles()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import androidx.fragment.app.FragmentActivity
import com.keylesspalace.tusky.TabData
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 {
val tab = tabs[position]
Expand Down

0 comments on commit 196ebdb

Please sign in to comment.