feat(tv): video-player auto-hide controls + play/pause initial focus + crimson palette#830
Merged
Ur-imazing merged 15 commits intomainfrom Apr 23, 2026
Merged
Conversation
Captures the requirements (5s auto-hide via D-pad, play/pause initial focus, crimson palette alignment) and the 7-unit implementation plan for apps/tv/src/components/VideoPlayer.tsx. Brainstorm: docs/brainstorms/2026-04-21-tv-video-player-controls-auto-hide-requirements.md Plan: docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md
…palette Unit 1 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md. Introduces the auto-hide state machine scaffolding in VideoPlayer.tsx and retires the file-local warm-salmon design tokens. This is foundation-only — subscriptions + refs are in place but the behavioral surface (fade, catcher, buffering/error handling) lands in Units 2-7. - Add controlsVisible, controlsFocusable, status, hasError, isScreenReaderEnabled, isReduceMotionEnabled state. - Add revealFocusPending + errorFocusPending one-shot focus flags (I6), mirroring Fix #5's clear-after-render pattern. - Add stable handler refs (scheduleHideRef, revealControlsRef) so subscriptions in Units 2-3 don't churn the native event emitter. - Mirror controlsVisible and isScreenReaderEnabled into refs so the useTVEventHandler callback in Unit 3 can read without re-binding. - Subscribe to AccessibilityInfo screenReaderChanged and reduceMotionChanged (HomeHero subscription shape). - Subscribe to AppState: on 'active', snap visible + rearm timer. - Claim hardware Menu on tvOS via TVEventControl.enableTVMenuKey() with menuKeyEnabledRef bookkeeping so cleanup is idempotent. - Clear inactivityTimerRef on unmount. - Move hasTVPreferredFocus from back pill to play/pause ({shouldRequestFocus || revealFocusPending}); back pill now receives {errorFocusPending} for Unit 5's error-state focus. - Retire the file-local warm-salmon tokens (ACCENT, ACCENT_ON, TEXT_PRIMARY, TEXT_SECONDARY) and swap every call-site to the shared Crimson Gallery COLORS.* tokens. Play button + progress fill are now COLORS.primary; icons + title use COLORS.text; subtitle + time readouts use COLORS.muted. All eight existing numbered fixes (#4, #5, #6, #8, #9, #15, #24, #25) are preserved. Typecheck + lint clean.
Unit 2 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md.
Implements the core hide/reveal cycle that Unit 1 scaffolded.
- Wrap topBar and controlsContainer in Animated.View, both bound to
the shared opacityAnim; collapsable={false} preserves z-order above
the Android TV VideoView surface.
- Add hideControls: setControlsFocusable(false) before the animation
so UIFocusEngine releases the controls before they're invisible
(I7 ordering), then 150 ms ease-out fade (or opacityAnim.setValue(0)
when reduce-motion is active), then flip controlsVisible to false.
- Add scheduleHide: idempotent timer arm — clears any in-flight timer,
then only re-arms a new 5 s if the state supports auto-hide (not
paused, not loading/error, no hasError, no screen reader).
- Add revealControls: early-return when already visible to neutralize
the catcher-onPress vs useTVEventHandler-select double-dispatch race
that Unit 3 introduces. Does NOT reset opacityAnim before animating
to 1 — in-flight hide animations reverse from their current mid-fade
value, avoiding a black flash.
- Assign scheduleHide / revealControls into their stable refs so the
AppState handler (Unit 1) and event handlers (Unit 3) see the
latest closure — mirrors Fix #15's onDismissRef.
- Modify playingChange listener: clear timer on pause, call
scheduleHideRef on resume. First isPlaying=true arms the D1 initial
5 s countdown.
- Add a 2 s mount fallback that calls scheduleHideRef unconditionally —
covers the race where autoplay retry succeeds but playingChange
hasn't fired yet. scheduleHide is idempotent so the normal path wins.
- Every control Pressable (back, rewind, play/pause, forward) now
reads focusable={controlsFocusable} and calls scheduleHide on
onFocus; the three active controls also call scheduleHide on
onPress after their original handler (satisfies D14).
Typecheck + lint clean.
…hrome
Unit 3 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md.
Closes the loop from Unit 2: hidden controls now have an owner for
D-pad input, so any key reveals the chrome without firing the focused
button's action.
- Add useTVEventHandler with a ref-stable useCallback (reads
controlsVisibleRef / isScreenReaderEnabledRef / revealControlsRef /
scheduleHideRef) so the native TV emitter doesn't re-register on
every render. Hidden state: any recognized event triggers reveal.
Visible state: Siri-remote swipes reset the timer for D14 (arrow /
Select events already reset via Pressable onFocus / onPress).
Defensive whitelist-or-fallback on the event-type string — names
vary across react-native-tvos versions and Siri-remote generations.
- Add BackHandler.addEventListener('hardwareBackPress'). Returns
true to consume. Hidden → revealControls. Visible → onDismissRef.
react-native-tvos bridges tvOS hardware Menu into this event, so a
single subscription covers both platforms. Paired with
TVEventControl.enableTVMenuKey from Unit 1 to keep Expo Router's
Stack from popping before our handler runs.
- Render an invisible Pressable catcher as the first child of
TVFocusGuideView when !controlsVisible && !isScreenReaderEnabled.
StyleSheet.absoluteFillObject lifts it above the sibling flex
layout; hasTVPreferredFocus claims focus on mount (fresh mount per
hide cycle); onPress={revealControls} is the primary Select path.
collapsable={false} preserves Android TV z-order above the
VideoView surface. accessibilityLabel + accessibilityRole for
App Store review.
- revealControls' existing early-return on controlsVisibleRef.current
already neutralizes the catcher-onPress vs useTVEventHandler-select
double-dispatch race (see Unit 2).
Typecheck + lint clean.
…ressable
Unit 4 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md.
The focus-restore wiring itself was landed across Units 1 and 2:
- U1 added revealFocusPending state + clearing useEffect + wired
hasTVPreferredFocus={shouldRequestFocus || revealFocusPending} on
play/pause.
- U2 added setRevealFocusPending(true) inside revealControls().
The plan offered three mitigations for tvOS's continuously-mounted
Pressable focus behaviour. We rely on mitigation (b) — per-cycle
focusable toggle — because Unit 2 already flips controlsFocusable
on every hide/reveal, producing a "new focus target" event for
UIFocusEngine. This comment records the choice so device QA knows
which path is in play and what the fallback is (mitigation (a):
a per-cycle `key` on the Pressable).
Unit 5 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md.
- Subscribe to player.addListener('statusChange', ...). Branches on
the expo-video VideoPlayerStatus literal union — confirmed from
node_modules as 'idle' | 'loading' | 'readyToPlay' | 'error' (there
is no 'buffering' or 'playing' value). Uses controlsVisibleRef and
seekTargetRef to avoid stale-closure reads inside the long-lived
callback.
- 'loading' + seekTargetRef.current !== null → no-op. Fix #4's seek
guard already handles the UI; without this branch the timer would
suspend (and force-reveal) on every 10 s skip press.
- 'loading' + no seek → clear timer, force reveal if hidden. A hidden
stall is indistinguishable from 'video ended' for a low-confidence
user; showing chrome during buffering gives them an affordance.
revealControls' early-return makes this a no-op if already visible.
- 'readyToPlay' → scheduleHideRef to restart the 5 s countdown.
- 'idle' → no-op (mount-time default, not a runtime transition).
- 'error' → setHasError(true), clear the timer permanently, snap
chrome visible, set errorFocusPending=true. hasError gates every
subsequent scheduleHide call (added to U2's guard list). Back pill
claims focus via hasTVPreferredFocus={errorFocusPending} (Unit 1).
- Replace titleRow contents with 'Playback failed — press Back to
exit.' when hasError. Inline, not a separate layer — preserves the
single-file / single-overlay constraint. Subtitle stays for context.
- Ghost rewind / play/pause / forward Pressables when hasError:
focusable={controlsFocusable && !hasError} plus a new
styles.controlDisabled (opacity 0.3) applied to each. The controls
remain mounted so the spatial layout doesn't collapse — only the
back pill is meaningful in the error state.
Typecheck + lint clean.
…smiss Unit 6 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md. - Modify the playToEnd listener so that when chrome is hidden at end-of-video, we synchronously snap it visible (setControlsVisible / setControlsFocusable / opacityAnim.setValue(1)) and dispatch onDismissRef via setTimeout(0). Intent is imperceptible technical continuity — the chrome appears for one paint so the dismiss transition doesn't start from a black-screen-with-no-UI. setTimeout(0) queues the dismiss after the current render commit; rAF's mapping to the native paint thread is less specified under react-native-tvos, so we avoid it here. Fallback to rAF if device QA shows a black frame on a hidden-to-dismissed transition. - Refactor to a doDismiss closure so both visible- and hidden-state paths share the Fix #24 try/catch wrapping. - Preserves I4a: playToEnd still always calls onDismissRef.current() — the new logic only delays the call by one tick when chrome was hidden, it never suppresses it. - Expand the comment on Unit 1's AppState handler to document the paused-state branch: scheduleHide's internal isPaused guard (U2) makes an explicit AppState-level guard unnecessary, which avoids re-subscribing AppState on every pause toggle. Also clarifies that playback resume behaviour is out of scope per the plan's deferred items. Typecheck + lint clean.
Unit 7 of docs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.md.
- Add a useEffect keyed on isScreenReaderEnabled that handles
mid-session SR toggles. SR→on: reveal chrome if hidden (the user
just gained access to controls they couldn't navigate) plus
AccessibilityInfo.announceForAccessibility('Player controls
visible') for audible confirmation. SR→off: rearm via
scheduleHideRef so a passive viewer who briefly toggled VO on-then-
off doesn't get stuck with permanent chrome. A srSeededRef guard
skips the first invocation so the mount-time seed from
AccessibilityInfo doesn't fire spurious side-effects.
- Add accessibilityLabel + accessibilityRole='button' to every
control Pressable:
back: "Back to {subtitle}" when a subtitle is present, else "Back"
rewind: "Rewind 10 seconds"
play/pause: dynamic — "Play" when paused, "Pause" when playing
forward: "Forward 10 seconds"
The invisible catcher (Unit 3) already carries "Show player
controls" + role=button.
- Unit 3's catcher conditional render ({!controlsVisible &&
!isScreenReaderEnabled && ...}) means the catcher is NEVER rendered
when a screen reader is active, so VoiceOver/TalkBack never
encounters a blank interactive element. Plus U2's scheduleHide
already bails on isScreenReaderEnabled, so auto-hide never fires
while SR is on.
Typecheck + lint clean.
Two fixes surfaced by manual QA on the Apple TV simulator: 1. Reveal-on-any-event was too aggressive. The hidden-state branch of useTVEventHandler triggered revealControls on ANY TV event, including the synthetic focus/blur/pan events that react-native-tvos fires when the engine reassigns focus. When the catcher mounted post-hide with hasTVPreferredFocus, the focus acquisition fired a synthetic event, the handler saw controlsVisibleRef.current === false, and called revealControls — producing an instant hide→reveal loop. Fix: strict-whitelist the eventType on the hidden-state branch. Only real directional (up/down/left/right), select, longSelect, and swipe* events count as user intent. Focus-change events are explicitly ignored. 2. Shorten the auto-hide inactivity period from 5000 ms → 3000 ms per user feedback (feels snappier during manual QA). Updated the three inline comments that cited the old value.
Discovered via /ce-code-review on the branch. All four break user- visible behaviour on playback transitions that manual QA on the first play cycle happened to miss. P1.1 — Stale-closure in scheduleHide broke resume-from-pause (adversarial 80, correctness+julik cross-corroborated). scheduleHide read isPaused/status/hasError from the render closure. After paused→ play, the onPress wrapper called scheduleHide synchronously before setIsPaused(false) committed, and the playingChange handler then called scheduleHideRef.current() also before the commit — both saw stale isPaused=true and bailed. Auto-hide never rearmed after any resume. Fix: mirror isPaused / status / hasError into refs (mirroring the existing controlsVisibleRef / isScreenReaderEnabledRef pattern). scheduleHide reads from refs. playingChange and statusChange handlers sync the relevant ref BEFORE calling scheduleHideRef so the guard reads the post-transition value. P1.2 — Animated.timing hide completion callback ran after state transitions (reliability rel-1 75, adversarial #4 60, julik-9 75). The callback unconditionally called setControlsVisible(false), which clobbered force-reveals triggered by error / AppState / unmount. Fix: capture the Animated.CompositeAnimation handle in hideAnimRef. Guard the completion callback with `if (finished)` — stopped animations carry finished=false. Call .stop() from revealControls, the error branch, the AppState handler, and the unmount cleanup so no stale completion can clobber a just-revealed chrome. P1.3 — 150 ms fade dead-zone dropped D-pad input (correctness 75, julik-1 75). During the fade, controls were non-focusable (set synchronously) but the catcher wasn't mounted yet (gated on !controlsVisible, which only flipped in the anim-completion). No focusable target → UIFocusEngine dropped input. Fix: gate the catcher on !controlsFocusable instead of !controlsVisible. The catcher mounts synchronously with the focusable flip, giving tvOS a valid focus target throughout the fade. P1.4 — AppState 'active' bypassed revealControls → orphaned focus (julik-5 75). The handler directly flipped state + opacity but never set revealFocusPending, so on foreground resume the catcher unmounted but play/pause had no hasTVPreferredFocus claim. Matches react- native-tvos #852. Fix: AppState handler now reads hasErrorRef and routes focus to the correct Pressable — errorFocusPending for error state (back pill), revealFocusPending for normal playback (play/pause). Also stops any in-flight hide animation so its completion doesn't clobber the force-reveal. Removed hasError from the effect's deps (reads via ref now — avoids AppState re-subscription on every hasError flip). Also includes P2.1 opportunistically: error branch of statusChange now explicitly setRevealFocusPending(false) so a same-tick reveal doesn't double-claim hasTVPreferredFocus against the back pill's errorFocusPending (adversarial #3 65 + julik-4 50). Typecheck + lint clean.
Two more fixes from /ce-code-review P2 tier. P2.2 — useTVEventHandler whitelist excluded hardware media keys (julik-3 50, correctness 25). The strict whitelist that fixed the hide→reveal loop in 2fd092c (excluding focus/blur/pan synthetic events) also excluded playPause / fastForward / rewind events that some Android TV remotes and Siri remote gen-1 emit. Users pressing the physical play/pause button while chrome was hidden saw nothing happen. Fix: denylist the four synthetic focus events (focus, blur, pan, panBegin, panEnd) instead of whitelisting a closed set of known user inputs. Anything not synthetic is treated as intent. Also route media-key events in the visible branch to scheduleHideRef so they reset the timer (same D14 behaviour as Siri swipes). P2.3 — Late-emission listeners could setState on an unmounted component (reliability rel-2 75, rel-3 75). Fix #24's try/catch only catches a throwing onDismiss — it doesn't cover the "callback fires after React unmount" case. Risks surface during rapid mount/unmount (e.g. quickly backing out of a video and opening a new one) when expo-video's native emitter delivers a queued statusChange / playing Change / playToEnd AFTER subscription.remove() has run. Fix: new isMountedRef, flipped to false in the existing unmount useEffect cleanup. Early-returns added to the statusChange + playingChange listener bodies and to the playToEnd hidden-chrome setTimeout(0) callback, so late native emissions are silently ignored instead of calling setState / onDismiss on a dead tree. Typecheck + lint clean.
|
🚅 Deployed to the forge-pr-830 environment in forge
|
…controls-ux # Conflicts: # apps/tv/src/components/VideoPlayer.tsx
…esh 3 related docs New learning (docs/solutions/design-patterns/): - rntvos-video-overlay-async-native-event-patterns-2026-04-23.md Four primary + two extension patterns extracted from the PR #830 code- review fix batch in apps/tv/src/components/VideoPlayer.tsx: ref-mirror + eager-sync for state-gated guards called from native callbacks; Animated.CompositeAnimation handle capture + if(finished) + .stop() on every transition; gate focus-trap catchers on synchronous focusability, not async visibility; set a one-shot hasTVPreferredFocus flag yourself on AppState foreground (react-native-tvos #852 workaround); denylist synthetic focus events in useTVEventHandler; isMountedRef for late- emission native callbacks. Cross-references added (ce-compound-refresh on 3 related best-practices docs — additions only, no rewrites): - react-native-tvos-porting-pitfalls-20260414.md: Related entry noting the new doc extends the pitfall catalog with async-event-vs-React-state issues that aren't covered by the six existing type/layout pitfalls. - playlist-video-player-sdui-mobile-20260409.md: inline "On TV, additionally" note in Section 5 (wasPlayingRef AppState guard) pointing to Pattern 4's one-shot hasTVPreferredFocus flag; Related entry for the TV companion. - tv-focus-driven-hero-patterns-20260420.md: Related entry linking Pattern 5's useTVEventHandler denylist as a complement to Section 6's onFocus-on-leaf rule.
CI's `format` job runs `prettier --check .` on the whole repo. The
brainstorm + plan were written by the ce-brainstorm / ce-plan skills,
which don't run prettier on *.md at write time. lint-staged only
covers *.{ts,tsx} + package.json on commit, so the drift slipped
past locally.
Pure formatting; no content changes.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Retires the warm-salmon Stitch mockup palette on the TV video player and introduces Netflix / YouTube-TV-style auto-hide controls to the custom overlay in
apps/tv/src/components/VideoPlayer.tsx. Auto-hide fires after 3 seconds of D-pad inactivity while playing (150 ms ease-out fade), any D-pad input reveals controls (100 ms ease-in), initial focus now lands on play/pause instead of back, and the full edge-case matrix is handled — buffering, playback error, video-end-while-hidden, app backgrounding, screen reader, reduce motion, Siri remote swipe, hardware media keys.The 8 pre-existing numbered fixes in
VideoPlayer.tsx(#4, #5, #6, #8, #9, #15, #24, #25) are all preserved.Single-file change to source code. No new dependencies. No CMS / GraphQL / deploy changes.
Key decisions (see brainstorm + plan)
ACCENT/ACCENT_ON/TEXT_PRIMARY/TEXT_SECONDARY) are deleted. All chrome now uses the shared Crimson GalleryCOLORS.*tokens — player matchesHomeHero.tsx,FocusableCard.tsx, and the rest of the TV app.BackHandler.hardwareBackPressis the unified Menu/Back channel (both tvOS + Android TV via react-native-tvos's bridge).useTVEventHandlerdenylists synthetic focus/blur/pan events on the hidden branch — anything else is treated as user intent, which means hardware media keys (playPause/fastForward/rewind) now reveal correctly.hasTVPreferredFocusflags (shouldRequestFocus,revealFocusPending,errorFocusPending) drive the correct control per transition.Origin artifacts
docs/brainstorms/2026-04-21-tv-video-player-controls-auto-hide-requirements.mddocs/plans/2026-04-22-001-feat-tv-video-player-auto-hide-controls-plan.mdBoth docs were reviewed twice by
ce-doc-reviewduring ideation.Commits (11)
docs(tv): brainstorm + plan for video-player auto-hide controlsfeat(tv): scaffold video-player auto-hide state + retire warm-salmon palette(U1)feat(tv): wire video-player auto-hide fade + 5s inactivity timer(U2)feat(tv): add D-pad catcher + event routing for hidden video-player chrome(U3)docs(tv): document U4 focus-restore mitigation choice on play/pause Pressablefeat(tv): handle video-player buffering + terminal playback error(U5)feat(tv): flash chrome for one paint before video-player playToEnd dismiss(U6)feat(tv): polish video-player accessibility for screen readers(U7)fix(tv): stop video-player auto-hide loop + shorten inactivity to 3s(QA fix — synthetic focus events caused hide→reveal loop)fix(tv): resolve 4 code-review P1 bugs in video-player auto-hide(see Code-review findings below)fix(tv): harden video-player listeners for late-emission + media keys(P2)Test plan
Existing convention in
apps/tv: no component-test harness (jest --passWithNoTests, two utility tests only). No unit tests introduced here. Manual couch QA is the verification story, with the 15 success criteria in the plan doc as the checklist.pnpm --filter @forge/tv typecheck— cleanpnpm --filter @forge/tv lint— cleangrep -nE "ffb3b0|410006|e9e1dd|a98987|\\bACCENT\\b|\\bACCENT_ON\\b|\\bTEXT_PRIMARY\\b|\\bTEXT_SECONDARY\\b" apps/tv/src/components/VideoPlayer.tsx— zero matchesEXPO_TV=1 npx expo run:ios— succeedsCode-review findings resolved
Ran
/ce-code-review(11 reviewer personas). Cross-corroborated findings were treated as blockers and fixed in commits 10 + 11:P1 (all fixed)
scheduleHideread gating values from render closure → stale after paused→play, buffering→ready, SR→off. Manual QA missed because the default-state first-play path worked.isPausedRef/statusRef/hasErrorRef; eager sync inplayingChange/statusChangehandlers.Animated.timinghide completion callback fired after unmount / error / AppState force-reveal, clobbering state.hideAnimRef;.stop()on reveal / error / unmount; guard completion withif (finished).!controlsFocusableinstead of!controlsVisible.revealControlsRef→ orphaned focus on foreground resume. Matches react-native-tvos #852.hasErrorRefand setserrorFocusPending/revealFocusPendingappropriately.P2 (fixed opportunistically)
hasTVPreferredFocusclaim when error + reveal commit same tick.setRevealFocusPending(false)in the error branch ofstatusChange.useTVEventHandlerstrict whitelist excluded hardware media keys.playPause/fastForward/rewindnow reveal.setStateon an unmounted component.isMountedRefflipped on unmount; early-returns instatusChange,playingChange, and theplayToEndhidden-statesetTimeout.Known residuals (deferred as P3)
These were surfaced by the review and intentionally deferred — no blockers for merge, but worth capturing for follow-up:
useAutoHideControlscustom hook to shrinkVideoPlayer.tsx(now ~1200 lines with 20+ hooks) and enable pure-reducer unit tests inapps/tv/src/lib/alongside the existingvalidateUrl.test.ts/easterDates.test.ts(maintainability maint-1).testIDto new Pressables + catcher (ce-agent-native) for UI automation / agent observability.scheduleHidegate onstatus === 'idle'so a stuck-idle stream (DNS/TLS hang before'loading') doesn't auto-hide chrome on a frozen screen (adversarial chore(tooling): enforce GitHub agent workflow #2).srSeededRefPromise race — if SR is toggled between seed Promise dispatch and resolution, the stale seed can swallow the real transition (adversarial chore(tooling): add missing repo config best practices #5, low incidence).Full review artifact:
.context/compound-engineering/ce-code-review/2026-04-23-115216-0ef2e72e/(per-reviewer JSON).Post-Deploy Monitoring & Validation
No production / runtime impact — this is a TV-app-only UI change, client-side, no server surface.
Validation window: next couch QA pass (Apple TV + Android TV). Failure signals would be user-reported: "controls don't hide" (→ P1.1 regression), "controls flicker on app return" (→ P1.2 regression), "first D-pad press after idle does nothing" (→ P1.3 regression). Watch for these in internal testing before promoting beyond the dev branch.
🤖 Generated with Claude Code