Repository navigation
Conversation
…n episode rail Home: while there is something to continue, the hero shows the time left instead of the description and the title switcher shares the action row, so the Continue watching rail starts on the first screen. Detail: one row of actions, play (a movie in progress says where it resumes, e.g. "Resume from 42:10"), Start over while in progress, favorite and a ⋯ menu with Mark watched/unwatched and Reset progress. Watched and remaining time come from the same position and duration as the progress bar, fixing "0 min watched" beside a filled bar. Series: episodes follow the actions as a horizontal rail with one focus stop per episode, a fixed 16:9 frame with progress or a watched check, and the selected episode's description and one Episode options button under it instead of a Watch actions picker under every card. Long seasons keep a window of about 24 episodes in the DOM. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4BLmp4uXpHf3o3owXfZ4e
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bundle ReportChanges will increase total bundle size by 667 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: iptvnator-rendererAssets Changed:
|
Contributor
 [Medium impact] The PR appears safe to merge, with a non-blocking focus issue when scrolling left through a long season. Findings
Fix with agent prompt### Issue 1
libs/tv/feature/src/episode-rail.tsx:139-149
Scrolling left of a focused later episode can lose focus. `ranges.map` puts each range in a separate nested array. When scrolling splits one range into two, the selected episode moves to the second array. Preact replaces its button despite `key={e.id}`, because the key now belongs to a different parent. The next remote arrow starts elsewhere on the page.
Flatten the ranges into one keyed child list so changing the visible window preserves each episode's button.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThe PR brings Continue watching higher on Home, groups rare detail actions into menus, and replaces the episode grid with a horizontal rail.
Reviews (3) · Last reviewed commit: "fix(tv): render scrolled-to episodes and..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The episode rail scrolled to its starting episode once, using the initial 480px step guess. Below 1400px the measured step is smaller, the leading spacer shrank and a long season could open on an empty rail. Repeat the placement when the step changes, until it is measured or the viewer moves. The design-review capture now restarts the sample with Watch again when it finished during the player screens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4BLmp4uXpHf3o3owXfZ4e
Owner
Author
|
Both Greptile findings were real. They are fixed in 4ba21a3:
Rechecked: episode-layout, vod-comfort, tmdb-series, tmdb-person, comfort-integration and presentation (39 passed), plus the design-review capture. Generated by Claude Code |
Pointer or wheel scrolling now moves a second, small window of rendered episodes, while the selected episode stays mounted. The rail test scrolls to the end of a 200-episode season and checks the last episode renders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4BLmp4uXpHf3o3owXfZ4e
Owner
Author
|
22faa5a addresses the remaining items:
Ran locally: the episode-layout, vod-comfort, tmdb-series, comfort-integration and design-review e2e suites (22 passed, 1 skipped), the app and spec typechecks, and lint. Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2249 +/- ##
========================================
Coverage 81.88% 81.88%
========================================
Files 1904 1905 +1
Lines 101617 101633 +16
Branches 24089 24852 +763
========================================
+ Hits 83204 83218 +14
Misses 11128 11128
- Partials 7285 7287 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Owner
Author
|
@greptile review |
Comment on lines
+139
to
+149
| {ranges.map(([from, to], part) => [ | ||
| from > (part ? ranges[part - 1][1] : 0) ? ( | ||
| <span | ||
| key={`spacer-${from}`} | ||
| class="episode-spacer" | ||
| aria-hidden="true" | ||
| style={{ | ||
| width: `${(from - (part ? ranges[part - 1][1] : 0)) * step}px`, | ||
| }} | ||
| /> | ||
| ) : null, |
Contributor
There was a problem hiding this comment.
Scrolling left of a focused later episode can lose focus. ranges.map puts each range in a separate nested array. When scrolling splits one range into two, the selected episode moves to the second array. Preact replaces its button despite key={e.id}, because the key now belongs to a different parent. The next remote arrow starts elsewhere on the page.
Flatten the ranges into one keyed child list so changing the visible window preserves each episode's button.
Prompt To Fix With AI
This is a comment left during a code review.
Path: libs/tv/feature/src/episode-rail.tsx
Line: 139-149
Comment:
**Scrolling drops episode focus**
Scrolling left of a focused later episode can lose focus. `ranges.map` puts each range in a separate nested array. When scrolling splits one range into two, the selected episode moves to the second array. Preact replaces its button despite `key={e.id}`, because the key now belongs to a different parent. The next remote arrow starts elsewhere on the page.
Flatten the ranges into one keyed child list so changing the visible window preserves each episode's button.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Render the episode window as one flat keyed list, so scrolling before a focused later episode no longer remounts it and drops focus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4BLmp4uXpHf3o3owXfZ4e
Owner
Author
|
0c279be fixes Greptile's "Scrolling drops episode focus" finding. The rail window is now rendered as one flat keyed list, so an episode keeps its button when the window splits or merges around it. The rail test now also scrolls back to the start, past a focused later episode (31 and 121 of 200), and checks that focus stays on that episode. The episode-layout, vod-comfort, tmdb-series and comfort-integration e2e suites pass (22 passed), along with the typecheck and lint. Generated by Claude Code |
Owner
Author
|
On 0c279be, "Performance journeys" fails on one counter:
It looks like noise of one idle tick. A re-run of the failed job should clear it. I'm not changing any baselines. Generated by Claude Code |
This branch has not been deployed
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
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.
Requested by Sergey · project thread
What changed
Steps 2–4 of the TV webOS design review (mockups A · Home and D · Detail), built on the tokens and focus system from #2237. Layout and styles only; the remote moves the same way, except that rare watch actions now sit behind one ⋯ press.
progressParts/progressSummaryderive "42 min watched · 1 h 12 min left" from the same position and duration as the bar. Anything under a minute reads "< 1 min". Cards and the hero use the same helper.episode-rail.tsx):apps/tv/src/detail-actions.css. The old episode-grid rules are removed fromtheme.css.Before / after
Same scenario before and after, on a private comparison page (ask for access if you can't open it): https://claude.ai/artifact/K8K8Rt6mMkJaCmJTCemcMJ. It covers screens 37, 08, 33, 10 and 11, plus two new ones: 38 (⋯ menu) and 39 (Episode options). Screen 05 (Home without history) does not change.
Not done here (deliberately)
Release note
.changes/tv-home-detail-episodes.mdChecks
tv/tv-testsunit tests pass (175), including a new regression test for the progress words (4 s of 27 s: "< 1 min watched · < 1 min left", "Resume from 0:04").release:notes:validateandtv:verify-buildpass.tv-e2ein Playwright Chromium: 229 passed, 1 skipped, 2 failed. I fixed and reran both:history"bounds each page …" failed once on the pager and passed 3 of 3 times on rerun. It does not touch the changed code.moreActionhelper.episode-layoutnow checks that the rail stays windowed and that a late still does not move the focused episode.tmdb-serieschecks that card height stays the same instead of the old 300/500px rows.home-featureasserts that the rail is on the first screen and that the resume hero has no description.docs/architecture/tv-application.md(detail actions, episode rail, resume hero).Generated by Claude Code