Skip to content

fix: multiple recyclerview adapter regressions in SubscriptionsFragment #7085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Bnyro merged 4 commits into from
Feb 10, 2025
Merged

fix: multiple recyclerview adapter regressions in SubscriptionsFragment #7085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
93092d4
Fix IndexOutOfBoundsException: Inconsistency detected. Invalid view h…
Pittvandewitt Feb 9, 2025
2e04192
Fix new videos not being added to feed
Pittvandewitt Feb 9, 2025
56d30ca
Remove obsolete sortedFeed variable
Pittvandewitt Feb 10, 2025
2085737
Display items from start
Pittvandewitt Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions app/src/main/java/com/github/libretube/ui/adapters/SubscriptionChannelAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import com.github.libretube.ui.viewholders.SubscriptionChannelViewHolder

class SubscriptionChannelAdapter :
ListAdapter<Subscription, SubscriptionChannelViewHolder>(DiffUtilItemCallback()) {
private var visibleCount = 20

override fun getItemCount() = minOf(visibleCount, currentList.size)

override fun onCreateViewHolder(
parent: ViewGroup,
Expand All @@ -31,13 +28,6 @@ class SubscriptionChannelAdapter :
return SubscriptionChannelViewHolder(binding)
}

fun updateItems() {
val oldSize = visibleCount
visibleCount += minOf(10, currentList.size - oldSize)
if (visibleCount == oldSize) return
notifyItemRangeInserted(oldSize, visibleCount)
}

override fun onBindViewHolder(holder: SubscriptionChannelViewHolder, position: Int) {
val subscription = getItem(holder.bindingAdapterPosition)

Expand Down
49 changes: 20 additions & 29 deletions app/src/main/java/com/github/libretube/ui/fragments/SubscriptionsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
private var isAppBarFullyExpanded = true

private var feedAdapter = VideosAdapter()
private val sortedFeed: MutableList<StreamItem> = mutableListOf()

private var selectedSortOrder = PreferenceHelper.getInt(PreferenceKeys.FEED_SORT_ORDER, 0)
set(value) {
PreferenceHelper.putInt(PreferenceKeys.FEED_SORT_ORDER, value)
Expand Down Expand Up @@ -175,15 +173,10 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub

if (viewModel.subscriptions.value != null && isCurrentTabSubChannels) {
binding.subRefresh.isRefreshing = true
channelsAdapter.updateItems()
binding.subRefresh.isRefreshing = false
}
}

binding.subFeed.addOnBottomReachedListener {
loadNextFeedItems()
}

// add some extra margin to the subscribed channels while the mini player is visible
// otherwise the last channel would be invisible
playerModel.isMiniPlayerVisible.observe(viewLifecycleOwner) {
Expand All @@ -192,8 +185,8 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
}
}

binding.channelGroups.setOnCheckedStateChangeListener { group, checkedIds ->
selectedFilterGroup = group.children.indexOfFirst { it.id == checkedIds.first() }
binding.channelGroups.setOnCheckedStateChangeListener { group, _ ->
selectedFilterGroup = group.children.indexOfFirst { it.id == group.checkedChipId }
if (isCurrentTabSubChannels) showSubscriptions() else showFeed()
}

Expand Down Expand Up @@ -231,25 +224,22 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
}
}

private fun loadNextFeedItems() {
private fun loadFeedItems(sortedFeed: List<StreamItem>) {
val binding = _binding ?: return

val hasMore = sortedFeed.size > feedAdapter.itemCount
if (viewModel.videoFeed.value != null && !isCurrentTabSubChannels && !binding.subRefresh.isRefreshing && hasMore) {
if (viewModel.videoFeed.value != null && !isCurrentTabSubChannels && !binding.subRefresh.isRefreshing) {
binding.subRefresh.isRefreshing = true

lifecycleScope.launch {
val toIndex = minOf(feedAdapter.itemCount + 10, sortedFeed.size)

var streamItemsToInsert = sortedFeed
.subList(feedAdapter.itemCount, toIndex)
.toList()

withContext(Dispatchers.IO) {
runCatching { streamItemsToInsert = streamItemsToInsert.deArrow() }
val streamItemsToInsert = sortedFeed.let {
withContext(Dispatchers.IO) {
runCatching { it.deArrow() }.getOrDefault(it)
}
}

feedAdapter.insertItems(streamItemsToInsert)
feedAdapter.submitList(streamItemsToInsert) {
binding.subFeed.scrollToPosition(0)
}
binding.subRefresh.isRefreshing = false
}
}
Expand Down Expand Up @@ -383,15 +373,13 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
val sorted = feed
.sortedBySelectedOrder()
.toMutableList()
sortedFeed.clear()
sortedFeed.addAll(sorted)

// add an "all caught up item"
if (selectedSortOrder == 0) {
val lastCheckedFeedTime = PreferenceHelper.getLastCheckedFeedTime()
val caughtUpIndex = feed.indexOfFirst { it.uploaded <= lastCheckedFeedTime && !it.isUpcoming }
if (caughtUpIndex > 0) {
sortedFeed.add(
sorted.add(
caughtUpIndex,
StreamItem(type = VideosAdapter.CAUGHT_UP_STREAM_TYPE)
)
Expand All @@ -404,13 +392,13 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
val notLoaded = viewModel.videoFeed.value.isNullOrEmpty()
binding.subFeed.isGone = notLoaded
binding.emptyFeed.isVisible = notLoaded
loadNextFeedItems()
loadFeedItems(sorted)

binding.toggleSubs.text = getString(R.string.subscriptions)

feed.firstOrNull { !it.isUpcoming }?.uploaded?.let {
PreferenceHelper.setLastFeedWatchedTime(it)
};
}
}

@SuppressLint("SetTextI18n")
Expand All @@ -423,9 +411,13 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub
)

if (legacySubscriptions) {
legacySubscriptionsAdapter.submitList(subscriptions)
legacySubscriptionsAdapter.submitList(subscriptions) {
binding.subFeed.scrollToPosition(0)
}
} else {
channelsAdapter.submitList(subscriptions)
channelsAdapter.submitList(subscriptions) {
binding.subFeed.scrollToPosition(0)
}
}

binding.subRefresh.isRefreshing = false
Expand All @@ -442,7 +434,6 @@ class SubscriptionsFragment : DynamicLayoutManagerFragment(R.layout.fragment_sub

fun removeItem(videoId: String) {
feedAdapter.removeItemById(videoId)
sortedFeed.removeAll { it.url?.toID() != videoId }
}

override fun onConfigurationChanged(newConfig: Configuration) {
Expand Down