Skip to content

[TV] Add the Starred Episodes screen - #5791

Merged
sztomek merged 9 commits into
mainfrom
feat/tv-starred-episodes
Aug 28, 2026
Merged

[TV] Add the Starred Episodes screen#5791
sztomek merged 9 commits into
mainfrom
feat/tv-starred-episodes

Conversation

@sztomek

@sztomek sztomek commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the Starred Episodes screen to the Android TV app, reaching parity with tvOS (UI/Starred/StarredEpisodesView.swift). The profile modal already rendered a "Starred Episodes" button, but it was wired to {} with a TODO — this PR builds the screen and wires the destination.

Behaviour mirrors tvOS:

  • Opened full-screen from Profile → Starred Episodes via the shared TvDetailOverlay (top bar hidden, Back returns to the tab layer).
  • When signed in, kicks a server refresh on open by enqueuing the existing StarredSyncWorker (which stars the returned episodes locally), then observes the local starred list reactively through EpisodeManager.findStarredEpisodesFlow(). Signed-out users see the local list only (the worker is a no-op when logged out). The list live-updates when episodes are starred/unstarred elsewhere.
  • Standard episode rows (TvEpisodeListItem), reusing the episode-actions modal (TvEpisodeActionsModal) and episode-info modal. View-only — there is no star/unstar action anywhere on TV, matching tvOS.
  • Empty state uses new tv_starred_empty_title / tv_starred_empty_subtitle strings (copy taken from tvOS).

A new TvEpisodeActionContext.Starred (source starred / EpisodeViewSourceType.Starred) attributes playback and episode-action analytics to the Starred screen, exactly as the podcast/search/playlist/up-next/now-playing contexts already do.

Analytics

  • Fires StarredShownEvent (no properties) when the screen appears — the Android equivalent of tvOS starred_shown — via eventHorizon.track(...), following the same pattern as TvUpNextViewModel.
  • Episode-action events (playback_play, etc.) flow through the shared managers/PlaybackManager with source = starred via the new action context.

Follow-up refinements (parity audit v2)

Three small ride-along commits from the v2 parity audit (Part 7):

  1. Default focus on open. The episode list now takes initial focus when the screen opens (requestInitialFocus = true), matching tvOS prefersDefaultFocus (StarredEpisodesView.swift) and the sibling overlay lists. Previously nothing was deterministically focused after the overlay appeared (the tab layer behind is focus-deactivated and the top bar is hidden).
  2. Bounded query. EpisodeDao.findStarredEpisodesFlow() now caps at LIMIT 1000 (ordered by last_starred_date DESC), matching tvOS (StarredEpisodesViewModel LIMIT 1000) and the existing playback-history query. Note: this DAO query is shared — the phone Starred list (ProfileEpisodeListViewModel) and Wear (StarredScreenViewModel) consume the same flow, so the 1000-row cap now applies on all three platforms, consistent with the history flow they already share. Unit tests mock the flow, so none are affected.
  3. Scroll survives Go-to-podcast. The LazyListState is hoisted to the always-composed TvStarredScreen so opening a podcast (which swaps the list out) and returning restores the scroll position; combined with (1), returning re-focuses the first visible row instead of landing at the top with nothing focused.

Fixes PCDROID-735 https://linear.app/a8c/issue/PCDROID-735/starred-episodes-screen

Testing Instructions

  1. Sign in on the TV app.
  2. Open the profile modal (top-right) → tap Starred Episodes.
  3. With starred episodes present: they appear as standard rows; SELECT plays and opens Now Playing; the more button opens the actions modal (no star/unstar option).
  4. Star/unstar an episode elsewhere and confirm the list live-updates.
  5. With none starred: the "No starred episodes" empty state shows.
  6. Sign out and reopen: only locally-starred episodes are shown (no server refresh).
  7. Press Back to return to the previous tab.

Screenshots or Screencast

Screen Analytics
Screenshot_20260827_140241 SCR-20260827-mnnn

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

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 5m 8s —— View job


Review

The view model and screen are faithful mirrors of TvUpNextViewModel/TvUpNextScreen and read fine. The problems are all in how the overlay is hosted: unlike every other TvDetailOverlay in the app, this one sits at the TvScaffold level outside TvScaffoldContent's Crossfade, so it survives tab changes and doesn't inherit the focus-handback the other call sites rely on. Two user-visible consequences below.

Blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:129 — Playing an episode from Starred never reveals Now Playing: openNowPlaying() only flips the tab underneath while the overlay stays on top with focus. Clear isStarredVisible in the openNowPlaying lambda (lines 63-68). (thread)
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:128 — No onHide, so after Back nothing regains D-pad focus (didFocusTopBar is already true, so autoFocusSelectedTab won't refocus the bar, and the profile modal that held focus is gone). Pass onHide = focusTopBar, matching the other seven overlay call sites. (thread)

Nits

  • tv/src/test/java/au/com/shiftyjelly/pocketcasts/starred/TvStarredViewModelTest.kt:109showing the screen only syncs when signed in only verifies isLoggedIn() was called; the check lives in the worker, not the view model, so it can't actually cover the signed-in path. (thread)

I did not build or run the test suite for this review — findings are from reading the diff and the surrounding TV navigation code.
· feat/tv-starred-episodes

@dangermattic

dangermattic commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator
1 Warning
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt
Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt
@sztomek sztomek added this to the 8.20 milestone Aug 27, 2026
@sztomek sztomek added [Type] Feature Adding a new feature. [Area] TV labels Aug 27, 2026
@sztomek
sztomek marked this pull request as ready for review August 27, 2026 12:38
@sztomek
sztomek requested a review from a team as a code owner August 27, 2026 12:38
@sztomek
sztomek requested review from geekygecko and removed request for a team August 27, 2026 12:38
@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 34s —— View job


Review

No new commits since my previous review (last commit cc0180f predates it), so this is the same diff moving from draft to ready. Both blocking findings are still open in TvScaffold.kt.

Blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:129 — still open: openNowPlaying (lines 63-68) doesn't clear isStarredVisible, so playing from Starred leaves the overlay on top of Now Playing. (thread)
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:128 — still open: the overlay passes no onHide, so nothing regains D-pad focus after Back. (thread)

Nits

  • tv/src/test/java/au/com/shiftyjelly/pocketcasts/starred/TvStarredViewModelTest.kt:113 — still open: showing the screen only syncs when signed in can't cover the signed-in path. (thread)
    · feat/tv-starred-episodes

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 2m 18s —— View job


Review

29b41c4 closes both earlier blocking findings — openNowPlaying now clears isStarredVisible, and the overlay passes onHide = focusTopBar — and renames the test per the nit. The onHide fix is too broad though: it also fires on the play path, where it races Now Playing for focus.

Blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:132onHide runs when openNowPlaying dismisses the overlay too, so the top bar's focus request competes with the player's seek-bar request and playing from Starred lands with focus on the tab strip; move the hand-back into onBack instead. (thread) Fix this →

Findings are from reading the diff and the surrounding TV focus code; I did not run the app or the test suite.
· feat/tv-starred-episodes

Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt Outdated
@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 3m 21s —— View job


Review

Three new commits since my last review: the scroll/initial-focus work in TvStarredScreen reads correctly — hoisting listState above the podcast branch and seeding initial focus from firstVisibleItemIndex keeps the two in sync on the round trip. The 1000-row cap landed on the shared DAO query rather than a TV-specific one, and the onHide focus finding is still open.

Blocking

  • modules/services/model/src/main/java/au/com/shiftyjelly/pocketcasts/models/db/dao/EpisodeDao.kt:278findStarredEpisodesFlow() also backs the mobile and Wear Starred screens, so the cap silently truncates them (and shrinks what mobile's "Select all" unstars); take the limit as a parameter and pass 1000 only from TV. (thread) Fix this →
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt:132 — still open: onHide = focusTopBar also fires on the play path. Now Playing only hides the top bar once isContentFocused is true (TvNowPlayingScreen.kt:210, :231), so the bar does compose and TvTabBar's retry loop takes focus off the player; move the hand-back into onBack. (thread)

Findings are from reading the diff and the surrounding TV focus code plus the other findStarredEpisodesFlow call sites; I did not build or run the app.
· feat/tv-starred-episodes

Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt Outdated
@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 1m 11s —— View job


Review

24a57d1 closes both open findings. findStarredEpisodesFlow now takes limit: Int = Int.MAX_VALUE threaded through EpisodeDaoEpisodeManagerEpisodeManagerImpl, so the mobile (ProfileEpisodeListViewModel.kt:65) and Wear (StarredScreenViewModel.kt:25) callers stay unbounded and only TvStarredViewModel passes 1000 — and the default-argument-on-an-abstract-DAO-method pattern already exists elsewhere in EpisodeDao (e.g. :198, :265). The focus hand-back moved from onHide into onBack, so it no longer fires on the openNowPlaying path. No new findings.

I read the diff and the surrounding TV focus / DAO call sites; I did not build the project or run the test suite.
· feat/tv-starred-episodes

@sztomek
sztomek merged commit 57ee7d0 into main Aug 28, 2026
23 checks passed
@sztomek
sztomek deleted the feat/tv-starred-episodes branch August 28, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Area] TV [Type] Feature Adding a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants