feat(web): right-click context menu, standalone hero carousel, instant tab navigation & UI layout fixes - #501
Conversation
…shifting on movie preview
…m margin to prevent overlap with play buttons
…ed catalog media cards
….org API endpoint
…e logos stay visible
… and fix text layout
…for instant navigation
…overview layout shifts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 473c244bd8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div style={{ display: !selected && section === "home" ? "contents" : "none" }}><HomeScreen /></div> | ||
| <div style={{ display: !selected && section === "search" ? "contents" : "none" }}><SearchScreen /></div> | ||
| <div style={{ display: !selected && section === "watchlist" ? "contents" : "none" }}><WatchlistScreen /></div> | ||
| <div style={{ display: !selected && section === "tv" ? "contents" : "none" }}><LiveTvScreen /></div> |
There was a problem hiding this comment.
Because this now mounts <LiveTvScreen /> even while it is hidden, its mount effect in LiveTvScreen.tsx lines 88-96 calls refreshIptv() whenever playlists exist and the snapshot is stale. For users with IPTV configured, simply opening Home now starts the expensive playlist refresh/parsing path in the background even if they never visit TV, regressing startup/home responsiveness; keep this screen lazy or pass visibility so the refresh only runs when the TV tab is active.
Useful? React with 👍 / 👎.
| type="button" | ||
| className={`context-menu-action-btn ${act.danger ? "is-danger" : ""}`} | ||
| onClick={() => { | ||
| act.action(); |
There was a problem hiding this comment.
This only invokes the supplied action, while the custom actions added for episode/season menus and Live TV rows do not call closeContextMenu() themselves. In those contexts, choosing actions such as Play Channel or Play Episode leaves the z-index 2000 scrim/card open over the player or source picker until the user dismisses it manually, unlike the default media actions which close first.
Useful? React with 👍 / 👎.
| markWatchedLocally({ mediaType: item.mediaType, id: item.id, season: seasonNumber, episode: episodeNumber }, !currentlyWatched); | ||
| setToast(!currentlyWatched ? "Marked as watched." : "Marked as unwatched."); | ||
|
|
||
| if (activeSyncProvider() !== "none") { |
There was a problem hiding this comment.
When a user is signed into ARVIO Cloud but has no Trakt/MDBList provider connected, this branch is skipped after only markWatchedLocally() runs, so the context-menu “Mark as watched” toast succeeds but the change is lost on refresh/reload and Cloud continue-watching can reappear. The existing detail-page flow persists the same action through saveProgress(...); this shared context-menu path needs the same Cloud write or it should not report a durable watched update.
Useful? React with 👍 / 👎.
ProdigyV21
left a comment
ProdigyV21
left a comment
There was a problem hiding this comment.
Thanks Himanth — there's real value here: the right-click/long-press context menu framework (pointer-only, so TV nav is untouched), the IPTV sort-order setting, and the skeleton/shimmer polish are all genuine improvements.
But this bundles 4+ features, and several collide with what landed on main this week:
-
Watched/Continue-Watching sync corrupts data.
toggleWatchedpassesprogress: 1tosaveProgress, but the cloud payload uses a 0–100 scale — cloud.ts only drops an item from Continue Watching atprogress >= 90, otherwise it inserts it at the TOP. "Mark as watched" therefore pushes the title into Continue Watching at 1% on every device, including the Android app.removeFromContinueWatchingalso never touches the cloud payload, so removed items resurface on the next refresh. -
The Live TV padding hunk re-breaks a landed fix. Main fixed the top-bar overlap at a measured
clamp(84px, 9vh, 108px)(e153ff58); this PR's 112–154px predates that and adds ~40px of dead space. Drop the hunk. -
The always-mounted AppShell is too costly on TV browsers (our primary target): all six screens mount at boot and hidden screens keep working — the hero keeps rotating and fetching logos/ratings forever. The
display: contentswrappers also break the.content > .screenrule. -
The hero rework regresses main's behavior: previous title's logo shows during swaps (main synchronizes those on purpose), hover/focus-to-pin is removed, and the 12 dots become D-pad focus stops.
-
Please remove:
version.json(build-generated, never commit), the TMDB host swap, the unused plugin/scraper types and settings fields, and the logo→PNG swaps.
Could you split it? PR 1: context menu with fixed watched/CW sync (+ role="dialog" on the scrim, CSS for the header-art/footer classes). PR 2: IPTV sort order — nearly mergeable as-is. PR 3: skeletons/shimmer. I'd gladly take all three.
|
Thanks for the detailed review! I've updated this PR to focus specifically on Context Menu & Watched/CW Sync:
Regarding point 3 (AppShell always-mounted for TV browsers) — why would people use the web app in a TV's browser if there is already the native app? |
|
Thanks for cleaning up the scope. The context-menu feature is a useful addition, and the current branch merges cleanly into main. Both GitHub CI and a local merged web production build pass. There are still a few behavior issues to fix before merging:
Minor cleanup: remove the trailing whitespace in Once these points are addressed, this should be in much better shape to merge. |
2 participants
Summary of Changes
This PR introduces a right-click context menu, converts the home hero banner into an auto-rotating carousel, resolves UI performance/lag bottlenecks on navigation and hover, and fixes layout clipping and alignment issues across the web app.
Detailed Breakdown of Issues, Causes, and Solutions
1. Standalone Auto-Rotating Hero Carousel & Hover Lag Elimination
onMouseEnterprefetch API calls and hero preview state updates for every passed card. Sweeping over 10–15 cards in a second spawned 30+ network requests and React state updates, saturating the main thread and causing noticeable cursor lag and UI stutter.MediaCard.tsx) so fast cursor sweeps across rails produce zero API calls or state thrashing.onTouchStart/onTouchEndwith a 50px threshold) for mobile.2. Instant Tab Navigation & Details Return (0ms Delay)
<HomeScreen />completely ({section === "home" && <HomeScreen />}). Re-entering Home forced component re-initialization, catalog state re-seeding, and network calls.selected ? <DetailsDrawer /> : <...screens...>) completely unmounted all active tab screens. Closing Details forced<HomeScreen />to re-mount from scratch, creating a ~1s loading delay.useEffectinHomeScreenblocked settingdisplayHerountil background title-treatment logo PNG/SVG requests (getLogoUrl) finished over the network.AppShell.tsxusing CSS display toggling (<div style={{ display: !selected && section === "home" ? "contents" : "none" }}>). Returning to Home or closing Details is now 100% instantaneous (0ms) with zero re-fetching or re-mounting.HomeScreento synchronously renderdisplayHeroimmediately whilegetLogoUrlfetches asynchronously in the background.3. Fixed Hero Overview Text Clipping & Vertical Layout Shifts ("Going Up & Down")
.hero-overviewused a rigid pixelheight: clamp(44px, 5.2vh, 60px) !important;combined with line-clamping and JS character truncation (desc.slice(0, 150)). On various viewport heights, line-height misalignments caused the 2nd/3rd line of text to be cut off horizontally ("tucked under").heightwasauto, overview descriptions with 1 or 2 lines shrank the text box height. This caused the Play & More Info buttons (.hero-actions) to jump up and down vertically when carousel slides auto-rotated between titles with different text lengths.desc.slice(0, 150)).min-height: calc(3 * 1.45em) !important; height: calc(3 * 1.45em) !important;on desktop (calc(2 * 1.45em)on mobile).4. Live TV Screen Top Clearance
.screen.livetv-shellwas targeted by apadding-top: 0CSS rule, causing the Live TV heading, search input, and playlist buttons to render at pixel 0 directly underneath the fixed top navigation bar..screen.livetv-shellfrompadding-top: 0and setpadding-top: clamp(112px, 14.3vh, 154px)inglobals.css, aligning Live TV top clearance with Home, Watchlist, Search, and Browse screens.5. GPU-Accelerated Shimmer & Skeleton Placeholders
background-positiongradient animations, causing frequent browser layout re-paints and scroll stutter.transform: translateX()compositor animations (will-change: transform) running smoothly at 60/120fps.6. PC Right-Click & Mobile Long-Press Context Menu
MediaContextMenucomponent and attached PC right-click (onContextMenu) and mobile long-press handlers across media cards.Verification
npx tsc --noEmit).