fix(tv): make the player controls reachable, and Center mean pause - #214
Conversation
Two things kept a viewer on the scrub bar from doing the obvious thing. D-pad Down is consumed by the activity-level remote-key bridge, which maps it to FocusTransport and asks the overlay to focus the transport row. That request is retried until observed — but the arrival test was `idleOverlayHasFocus`, hasFocus on the overlay ROOT, which is already true whenever any control has focus. With the scrub bar focused the retry loop concluded focus had arrived and never requested, so Down did nothing. It also explains the workaround: Back hides the controls, clearing the flag, so the next Down finally requested. Observe focus per control row and test the row actually asked for. Center on the bar entered a scrub mode. The Google TV remote has no play/pause key, so that spent the viewer's only one-press pause on something Left/Right already do (skip, and long-press auto-seek). Center now lands any scrub in flight and then toggles playback: racing forward it stops on the frame you asked for; hunting a spot while paused it plays on from it. Verified on a Google TV Streamer: scrub bar -> Down now focuses play/pause in one press. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 59 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe TV player overlay now tracks scrubber and transport focus independently. Focus requests validate the requested row. The scrubber commits active seeking and invokes playback toggling on Center or Enter. ChangesTV player focus and playback controls
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant DPad
participant TvPlayerIdleOverlay
participant TvPlayerScrubber
participant TvPlayerTransportControls
TvPlayerScrubber->>TvPlayerIdleOverlay: report scrubber focus
TvPlayerTransportControls->>TvPlayerIdleOverlay: report transport focus
DPad->>TvPlayerScrubber: send Center or Enter
TvPlayerScrubber->>TvPlayerScrubber: commit active scrub
TvPlayerScrubber->>TvPlayerIdleOverlay: invoke onPlayPause callback
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Center commits the in-flight scrub and then toggles playback. Solo that applies locally and in order, but in a Watch Together room they are two independently launched requests and the play/pause carries the LIVE position rather than the committed one — so it can land after the seek and pull every participant back to where the scrub started. Skip the toggle when a room owns transport; Center there commits, as it did before. The surface is compile-time disabled (CLIENT_WATCH_TOGETHER_SURFACE_ENABLED = false), so this is latent rather than reachable, but the ordering hazard is real. Found by Codex review of Silo-Server#214. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Two things stopped a viewer on the scrub bar from doing the obvious thing. Both were found on a Google TV Streamer and the fixes were verified there.
D-pad Down did not reach the controls
Down inside the player is consumed by the activity-level remote-key bridge, which maps it to
FocusTransportand asks the overlay to focus the transport row. That request is retried until observed — but the arrival test wasidleOverlayHasFocus, which ishasFocuson the overlay root, and that is alreadytruewhenever any control holds focus, the scrub bar included. The retry loop therefore concluded focus had already arrived and never issued a request, so Down did nothing at all.It also explains the workaround people found: Back hides the controls, which clears the flag, so the next Down took the "controls hidden" branch and genuinely requested focus. Hence "hit Back, then Down".
Focus is now observed per control row, and the retry loop tests the row it actually asked for.
Center on the bar entered a scrub mode instead of pausing
The Google TV remote has no dedicated play/pause key, so Center with the overlay up is the only one-press pause a viewer has. It was bound to entering a scrub mode — something Left/Right already cover (skip, and long-press auto-seek).
Center now lands any scrub in flight and then toggles playback. Racing forward it stops on the frame you asked for; hunting a spot while paused it plays on from it. Same key, one meaning.
Verification
:androidTvApp:testDebugUnitTestpasses.🤖 Generated with Claude Code
https://claude.ai/code/session_01Q9UyH5fvug685dzUFLtdpW
Summary by CodeRabbit