Feat/featured stream#4335
Conversation
Configurable, OF-owned 'now streaming' panel that shows only while a configured Twitch channel is actually live, then hides. Set STREAM_CHANNELS (comma list of channel logins) to enable; empty/unset = off (renders nothing, zero impact). First live channel wins, so OF can list its own channel for releases + the OFM channel for tournaments. - Client-side live detection via the Twitch Embed SDK (no backend, no API secret); reads initial state with getEnded() on READY to handle the offline-on-load quirk, ONLINE/ OFFLINE for transitions. parent=[location.hostname] self-adapts to any domain/subdomain. - Muted autoplay (browser policy); hides while in a lobby/game; dismissable. - Config flows STREAM_CHANNELS env -> ServerEnv -> RenderHtml -> BOOTSTRAP_CONFIG -> ClientEnv, mirroring the existing instanceId pattern. New Lit component FeaturedStream (light DOM + Tailwind), placed next to homepage-promos. i18n added. Passes tsc, eslint, prettier, EnJsonSorted.
index.html is EJS-rendered twice: by vite-plugin-html at dev/build time and by the server's RenderHtml at runtime. The dev pass reads its locals from htmlAssetData in vite.config, which was missing streamChannels -> 'streamChannels is not defined' on `npm run dev`. Add it (reads env.STREAM_CHANNELS, default empty), mirroring instanceId.
…treaming) - z-[45000] so it overlays the footer (relative z-50) and page content, not behind it. - Larger: ~40vw (clamp 340-720px) bottom-right, a good chunk of the corner. - Muted autoplay by default (unpaused); nudges play() when shown/expanded. - No close button — minimize only. Minimized clips the card to the header bar via overflow-hidden + max-height; the iframe stays mounted and streaming in the background, so un-minimizing is instant. Header shows LIVE + the channel name as the title.
…orce-mute - Minimized was clipping the iframe to a header bar, which Twitch treats as off-screen and pauses. Now minimized = a small but visible corner thumbnail, so the player stays in-viewport and keeps streaming; expand returns it to full size. - kickPlay no longer calls setMuted(true): respect the user's mute choice (muted on initial autoplay, but if they unmute it stays unmuted, including while minimized).
- Header now shows the live stream TITLE (the embed only exposes channel name), fetched from decapi (no auth, CORS-open), with graceful fallback to the channel name. Production can swap to a Helix-backed endpoint. - Minimized was 256px — below Twitch's ~340px minimum player size, which made it pause. Bumped to 360px and call play() on every toggle, so it keeps streaming when minimized. - Renamed the title state to streamTitle (HTMLElement.title is reserved on custom elements).
- Header is a drag handle (touch + mouse); on release the panel snaps to the nearest screen corner and remembers it (localStorage). Works expanded or minimized. - Title is now plain text so the whole header is grabbable (watch via the embed itself). - Investigated hiding the stream while minimized (opacity/offscreen/cover): all VIOLATE Twitch's Developer Agreement (embeds must not be obscured; Twitch enforces visibility and will disable playback / can revoke embed access). Not worth it for an official integration — minimized stays a visible thumbnail (>=340px so Twitch keeps it playing).
Corner was already saved to localStorage; do the same for the minimized/expanded state so the panel comes back the way the player left it.
The header now doubles as a click target: a plain click opens twitch.tv/<channel> in a new tab, while a drag (past a 5px threshold) still snaps to a corner. The minimize button is excluded, and the Twitch player is a separate iframe so its own controls are untouched.
Move the featured-stream channel config off build-time env vars onto a served JSON (featured-stream.json) with a bundled fallback, mirroring the news.json pattern: getFeaturedStream() fetches the API-hosted config, Zod-validates it, and falls back to the bundled OFF config on any failure. OF can flip the panel on/off and set channels without a redeploy. Hardening from a self-review pass: - Validate channel logins (Twitch login regex) so a typo fails the config closed instead of flowing into the embed/link. - Render <featured-stream> as a direct <body> child, not inside the in-[.in-game]:hidden home container, so the Twitch iframe is never display:none'd mid-game (which would unmount the player). The component is position:fixed and self-hides + pauses during a game (ToS-clean). - Mount-generation guards so events from a superseded player are ignored (no double-advance / stale-title races); destroy() the old player on teardown; re-probe every 60s when all channels are offline so the panel still appears when a stream goes live later. - Keyboard-accessible title control; pause-in-game; respect user mute. Tests: schema (defaults, bad-channel rejection), getFeaturedStream fetch/validate/fallback paths, cornerFromCenter quadrants.
The broadcast title already fell back to the channel name on failure; make that robust and easy to upgrade: - 5s AbortController timeout so a hung proxy request can't dangle. - Pull the title URL into titleEndpoint() so OF can later point it at a Helix-backed route on their own API (same plaintext contract) without touching the fetch/fallback logic. The channel name is shown immediately and only replaced on a clean 2xx response for the current channel, so down/rate-limited/slow/not-found all degrade to the channel name.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a ChangesFeatured Twitch Stream Panel
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Page as index.html
participant Main as Main.ts
participant FeaturedStream
participant API as getFeaturedStream()
participant SDK as Twitch Embed SDK
Page->>Main: load application scripts
Main->>FeaturedStream: register custom element
Page->>FeaturedStream: initialize element
FeaturedStream->>API: request stream configuration
API-->>FeaturedStream: return validated or fallback configuration
FeaturedStream->>SDK: load Twitch player SDK
SDK-->>FeaturedStream: provide Twitch.Player
FeaturedStream->>SDK: mount configured channel
SDK-->>FeaturedStream: report channel status
FeaturedStream-->>Page: render or hide stream panel
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/client/components/FeaturedStream.test.ts (2)
106-112: ⚡ Quick winAdd boundary tests for center-line tie behavior in
cornerFromCenter().Line 107 covers quadrants well. Add exact-midpoint cases too, since current logic uses strict
>and ties resolve to top/left.Suggested diff
describe("cornerFromCenter", () => { it("maps each quadrant to the nearest corner", () => { expect(cornerFromCenter(100, 100, 1000, 800)).toBe("tl"); expect(cornerFromCenter(900, 100, 1000, 800)).toBe("tr"); expect(cornerFromCenter(100, 700, 1000, 800)).toBe("bl"); expect(cornerFromCenter(900, 700, 1000, 800)).toBe("br"); }); + + it("resolves center-line ties consistently", () => { + expect(cornerFromCenter(500, 400, 1000, 800)).toBe("tl"); + expect(cornerFromCenter(900, 400, 1000, 800)).toBe("tr"); + expect(cornerFromCenter(500, 700, 1000, 800)).toBe("bl"); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/components/FeaturedStream.test.ts` around lines 106 - 112, The current test for cornerFromCenter() only covers clear quadrant cases but misses boundary conditions. Add additional test cases within the same describe block to test exact midpoint scenarios where coordinates fall on the center lines. Specifically, add tests for when x equals exactly half the width (500 for width 1000), when y equals exactly half the height (400 for height 800), and when both coordinates are at their respective midpoints. Based on the strict comparison operators in the function logic, verify that these tie cases resolve to the top-left corner as expected by the implementation.
71-83: ⚡ Quick winAssert the fetch call contract, not only the returned value.
Line 74 verifies output, but it does not lock URL/header behavior. A small assertion here will catch request-contract regressions earlier.
Suggested diff
- const stubFetch = (impl: () => unknown) => - vi.stubGlobal("fetch", vi.fn(impl)); + const stubFetch = (impl: () => unknown) => { + const fetchMock = vi.fn(impl); + vi.stubGlobal("fetch", fetchMock); + return fetchMock; + }; it("returns the served config on HTTP 200 with valid JSON", async () => { - stubFetch(() => + const fetchMock = stubFetch(() => Promise.resolve({ status: 200, json: async () => ({ enabled: true, channels: ["eslcs"] }), }), ); const cfg = await getFeaturedStream(); expect(cfg).toEqual({ enabled: true, channels: ["eslcs"] }); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/featured-stream.json"), + expect.objectContaining({ + headers: expect.objectContaining({ Accept: "application/json" }), + }), + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/components/FeaturedStream.test.ts` around lines 71 - 83, The test in the "returns the served config on HTTP 200 with valid JSON" block currently only verifies the returned value from getFeaturedStream() with the expect(cfg).toEqual() assertion, but does not verify that the fetch function was called with the correct URL and headers. Add assertions after the getFeaturedStream() call to verify that the stubbed fetch function was invoked with the expected URL and request headers to ensure the request contract is properly tested and catch any regressions in how the function constructs its HTTP request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/client/components/FeaturedStream.test.ts`:
- Around line 106-112: The current test for cornerFromCenter() only covers clear
quadrant cases but misses boundary conditions. Add additional test cases within
the same describe block to test exact midpoint scenarios where coordinates fall
on the center lines. Specifically, add tests for when x equals exactly half the
width (500 for width 1000), when y equals exactly half the height (400 for
height 800), and when both coordinates are at their respective midpoints. Based
on the strict comparison operators in the function logic, verify that these tie
cases resolve to the top-left corner as expected by the implementation.
- Around line 71-83: The test in the "returns the served config on HTTP 200 with
valid JSON" block currently only verifies the returned value from
getFeaturedStream() with the expect(cfg).toEqual() assertion, but does not
verify that the fetch function was called with the correct URL and headers. Add
assertions after the getFeaturedStream() call to verify that the stubbed fetch
function was invoked with the expected URL and request headers to ensure the
request contract is properly tested and catch any regressions in how the
function constructs its HTTP request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 527dab0c-0f46-4315-a2b6-cd0f72c8c1b3
📒 Files selected for processing (8)
index.htmlresources/featured-stream.jsonresources/lang/en.jsonsrc/client/Api.tssrc/client/FeaturedStream.tssrc/client/Main.tssrc/core/ApiSchemas.tstests/client/components/FeaturedStream.test.ts
|
This pull request is stale because it has been open for fourteen days with no activity. If you want to keep this pull request open, add a comment or update the branch. |
evanpelle
left a comment
There was a problem hiding this comment.
Claude reivew:
In-game recheck bug (blocker): onJoin pauses the player but doesn't cancel recheckTimer, and mountPlayer/advance don't check inGame. If a recheck fires mid-game and the channel is live, a new autoplaying player streams behind the opacity-0 panel — the obscured-embed case the comments say we avoid. Cancel the timer on join, restart probing on leave.
Schema alignment with the infra PR: client requires {3,25} and fails the whole config closed on one bad entry; infra accepts {1,25}. As-is, an admin can save a "valid" config that silently turns the feature off for every client. Let's tighten infra to match (and consider filtering bad entries client-side instead of rejecting everything).
Fetching the broadcast title from decapi.me sent each viewer's IP to a third-party proxy. Remove it; the panel shows the channel name instead. The stream title comes back later via the official Twitch API (Helix) served from our own backend, so no client-side third-party call.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/FeaturedStream.ts`:
- Line 363: Update the relevant FeaturedStream rendering flow around the
channel-name span to restore the reactive streamTitle state and its
broadcast-title fetch/population path, then render streamTitle when available
and channel as the fallback. Ensure the displayed value remains reactive and
preserves channel-only behavior when no title is returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e51aa85b-881c-4b9e-a4d4-252c805618a2
📒 Files selected for processing (1)
src/client/FeaturedStream.ts
…ead of failing closed Addresses review feedback: - In-game recheck bug: onJoin cancels the recheck timer and mountPlayer bails while inGame, so a recheck (or a channel coming online) can no longer mount an autoplaying player behind the hidden panel (obscured embed / Twitch ToS). onLeave re-probes. - Config resilience: FeaturedStreamSchema drops invalid channel entries individually instead of rejecting the whole config, so one bad login can't silently disable the feature for every client. Valid channels still flow through.
Add approved & assigned issue number here:
Resolves #4333
Description:
Adds a small homepage panel that embeds a live Twitch stream, shown only while a configured channel is actually live. When nothing configured is live, it renders nothing.
Config-gated and off by default. The client reads a served config like news.json: an on/off toggle plus the Twitch channel(s), with a bundled fallback so it stays off until the backend serves config. So OF admins control whether it is on and which channel is linked, no redeploy. First live channel in the list wins.
You can drag the panel between the corners or minimise it, both persisted across refresh. Clicking it opens the channel on Twitch. It shows the live broadcast title, fetched from a third party with a clean fallback to the channel name if that is unavailable.
Mainly designed OFM tournament streams, but OF can point it at its own channel for major releases.
Notes:
featured-stream.json), which I will follow up on the infra side.Showcase of how it would look like:
https://github.com/user-attachments/assets/4874bcd6-e7e8-49d8-94ab-20512ab1f71c
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
zixer._