Skip to content

Fix Podcasts tab sort scroll drift - #5516

Draft
joashrajin wants to merge 3 commits into
mainfrom
codex/fix-podcasts-sort-scroll
Draft

Fix Podcasts tab sort scroll drift#5516
joashrajin wants to merge 3 commits into
mainfrom
codex/fix-podcasts-sort-scroll

Conversation

@joashrajin

Copy link
Copy Markdown
Contributor

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

  1. Open the Podcasts tab and make sure the list is at the top.
  2. Open the Podcasts sort options.
  3. Change sorting from Date Added to Episode Release Date.
  4. Verify the first row remains aligned at the top and the list does not appear silently scrolled down.
  5. Scroll down in the Podcasts tab, change the sort order again, and verify the app does not force-scroll you to the top.
  6. Run JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" ./gradlew :modules:features:podcasts:testDebugUnitTest.
  7. 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
  • for accessibility with TalkBack

@joashrajin joashrajin added [Type] Bug Not functioning as intended. [Area] Podcasts Tab labels Jul 1, 2026
@joashrajin joashrajin added this to the 8.16 milestone Jul 1, 2026
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished @joashrajin's task in 3m 18s —— View job


PR Review — Fix Podcasts tab sort scroll drift

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

  1. 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).

  2. CHANGELOG (nit) — this is a user-facing bug fix (Changing Sort order causes the page to scroll down #2121); the checklist item is unchecked. Consider adding a CHANGELOG.md entry.

  3. 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

@dangermattic

Copy link
Copy Markdown
Collaborator
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@joashrajin joashrajin self-assigned this Jul 2, 2026
@wpmobilebot wpmobilebot modified the milestones: 8.16, 8.17 Jul 6, 2026
@wpmobilebot

Copy link
Copy Markdown
Collaborator

Version 8.16 has now entered code-freeze, so the milestone of this PR has been updated to 8.17.

@wpmobilebot wpmobilebot modified the milestones: 8.17, 8.18 Jul 21, 2026
@wpmobilebot

Copy link
Copy Markdown
Collaborator

Version 8.17 has now entered code-freeze, so the milestone of this PR has been updated to 8.18.

@wpmobilebot wpmobilebot modified the milestones: 8.18, 8.19 Aug 3, 2026
@wpmobilebot

Copy link
Copy Markdown
Collaborator

Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Area] Podcasts Tab [Type] Bug Not functioning as intended.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing Sort order causes the page to scroll down

3 participants