You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixes the Podcasts tab sort-change scroll drift by preserving the top position only when the user starts at the top of the list. The RecyclerView is pinned back to offset 0 after the sorted adapter update commits, and the PodcastsViewModel now pairs each emitted podcast list with the sort type that produced it so Recently Played cannot briefly show mismatched state.
Open the Podcasts tab and make sure the list is at the top.
Open the Podcasts sort options.
Change sorting from Date Added to Episode Release Date.
Verify the first row remains aligned at the top and the list does not appear silently scrolled down.
Scroll down in the Podcasts tab, change the sort order again, and verify the app does not force-scroll you to the top.
Run JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" ./gradlew :modules:features:podcasts:testDebugUnitTest.
Run JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" ./gradlew spotlessCheck.
Screenshots or Screencast
Not attached. This is a small UI behavior fix for RecyclerView scroll position; please add a short before/after screencast from a device if visual proof is required.
Checklist
If this is a user-facing change, I have added an entry in CHANGELOG.md
Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
I have considered whether it makes sense to add tests for my changes
All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
Any jetpack compose components I added or changed are covered by compose previews
I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.
I have tested any UI changes...
with different themes
with a landscape orientation
with the device set to have a large display and font size
I reviewed the diff (FolderAdapter, PodcastsFragment, PodcastsOptionsDialog, PodcastsViewModel, and the new test). Overall this is a well-scoped, thoughtful fix. The two-part approach — (1) pairing each emitted podcast list with the sort type that produced it in the ViewModel, and (2) pinning the RecyclerView to offset 0 in submitList's commit callback only when the user started at the top — is the right shape and cleanly addresses the mismatched-state flicker.
Gather context (read changed files)
Analyze scroll-position logic
Analyze ViewModel list/sort pairing change
Review tests
What looks good
ViewModel pairing (PodcastsViewModel.kt): moving the sort type into podcastsFlow via .map { sortType to it } and dropping the separate settings.podcastsSortType.flow combine arg is the correct fix. Previously the settings flow could emit RECENTLY_PLAYED while flatMapLatest still held the old latest-episode list (replay=1), producing a mismatched frame. Now the sort type can never lead the list. The new test guards exactly this regression (expectNoEvents() before the recently-played list emits).
currentSortType = uiState.folder?.podcastsSortType ?: uiState.sortType correctly distinguishes the folder-detail view (folder's own sort) from the root list (settings sort).
keepRecyclerViewAtTopAfterSort only pins when canScrollVertically(-1) == false, so scrolled-down users keep their position (test step 5). The == false form also null-safes a missing binding into the "don't scroll" default. Good.
scrollRecyclerViewToTop casts to LinearLayoutManager, which covers both GridLayoutManager and the list-view LinearLayoutManager, so scrollToPositionWithOffset(0, 0) works for every layout mode.
Guards (sort != folder.podcastsSortType, order.clientId != sortOrder.clientId) avoid pinning when re-selecting the current sort. onDestroyView resets the flag.
Findings
Rapid successive sort changes can drop a pending scroll (minor) — flagged inline on PodcastsFragment.kt. The commit callback unconditionally runs sortTypeToKeepAtTop = null, which can wipe a newer pending sort set between emissions. Guarding the clear with if (sortTypeToKeepAtTop == pendingSortType) fixes it. Low severity (only fast double-taps).
Test coverage note (nit) — the new test nicely covers the ViewModel pairing, but the actual scroll/pin behavior in PodcastsFragment is untested. That's understandable given RecyclerView/fragment test cost; just calling it out. The core regression is the ViewModel one and it is covered.
Verification
I reviewed statically and did not run :modules:features:podcasts:testDebugUnitTest or spotlessCheck in this environment — please confirm both pass locally (per your testing instructions) before merge.
Nothing here is blocking; #1 is the only substantive item and it's a low-severity edge case. Nice fix. 👍
• Branch: codex/fix-podcasts-sort-scroll
Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the Podcasts tab sort-change scroll drift by preserving the top position only when the user starts at the top of the list. The RecyclerView is pinned back to offset 0 after the sorted adapter update commits, and the PodcastsViewModel now pairs each emitted podcast list with the sort type that produced it so Recently Played cannot briefly show mismatched state.
Fixes #2121
Testing Instructions
JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" ./gradlew :modules:features:podcasts:testDebugUnitTest.JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" ./gradlew spotlessCheck.Screenshots or Screencast
Not attached. This is a small UI behavior fix for RecyclerView scroll position; please add a short before/after screencast from a device if visual proof is required.
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...