fix(live-news): call destroyPlayer() before offline/embed error message (fixes #347)#388
Conversation
…ge (fixes koala73#347) Made-with: Cursor
|
@Niboshi-Wasabi is attempting to deploy a commit to the Elie Team on Vercel. A member of the Team first needs to authorize it. |
Made-with: Cursor
|
Thanks for this PR @Niboshi-Wasabi! We reviewed both fixes: Fix 1: Live News black screen — ✅ Looks greatThe root cause analysis is spot on. We'll take this fix and merge it separately so users benefit from it immediately. Fix 2: canvas-confetti CDN loading — Needs reworkA few issues we found:
We recommend splitting this PR: we'll merge Fix 1 now, and if you'd like to submit Fix 2 separately with the promise caching fix and a clearer description of when the error occurs, we're happy to review it. |
Made-with: Cursor
|
Hi @koala73, Thanks for the review. We’ve reverted Fix 2 (canvas-confetti) from this PR, so the current PR only contains Fix 1 (Live News panel black screen). On the canvas-confetti import failure: in our case it failed when the package was not present in node_modules — for example right after cloning the repo without running So we’re fine with the current approach (require Thanks again for reviewing. |
Code ReviewThe core fix (LiveNewsPanel.ts) is correct and minimal. Adding Requests before merge:
Verdict: Core fix is sound — just remove the two docs files and ideally revert the celebration.ts cosmetic change, then this is good to merge. |
… original - Remove docs/FIX2_CANVAS_CONFETTI_REWORK.md and docs/PR_347_BODY.md (working notes, not for repo) - Revert celebration.ts to original void confetti(...) calls (no run() helper); keeps PR focused on koala73#347 fix Made-with: Cursor
|
Deployed in #410 (comment) |
|
thank you @Niboshi-Wasabi |
Summary
1. Live News panel black screen (Issue #347)
Fixes the bug where the Live News panel stays black after switching from an offline channel back to a live channel. The root cause was that
showOfflineMessage()andshowEmbedError()replaced the panel content without callingdestroyPlayer(), sothis.playerremained set. When switching back to a live channel,initializePlayer()returned early and no new YouTube player was created. This PR callsdestroyPlayer()at the start of both methods so that a new player is created when returning to a live stream.2. canvas-confetti import resolution
Resolves the Vite error
Failed to resolve import "canvas-confetti"when the package is missing fromnode_modules(e.g. fresh clone). The celebration service now loads confetti at runtime via a CDN script instead of bundling the package, so the app builds and runs without requiringcanvas-confettito be installed.vite.config.tsincludesoptimizeDeps.include: ['canvas-confetti']for environments where the package is present.Type of change
Affected areas
/api/*)Checklist
api/rss-proxy.jsallowlist (if adding feeds)npm run typecheck)Screenshots
No screenshots. #347: Reproduce by adding an offline channel → switch to it → switch back to a live channel; the stream now loads instead of staying black. Confetti: App builds and runs without
canvas-confettiin node_modules; celebrations load confetti from CDN when used.