From 884371b30180da46c31435767335144072ea8163 Mon Sep 17 00:00:00 2001 From: o-p-o-p-o <204936872+o-p-o-p-o@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:53:55 +0100 Subject: [PATCH 1/5] =?UTF-8?q?fix(debates):=20stop=20debate=20videos=20si?= =?UTF-8?q?tting=20on=20"Loading=E2=80=A6"=20forever?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useDebatePlayback claimed its "already fetched" key before the signed-URL request settled. When a cleanup landed mid-flight — StrictMode's dev double-run, or scrolling a card away and back before the fetch settled — the re-run skipped as already fetched while the cancelled run discarded its URLs, leaving the card on the placeholder. Claim the key only once URLs are committed. Re-activating a card that already holds URLs still does not refetch (GEO-2895). Co-Authored-By: Claude Opus 5 (1M context) --- .../core/debates/use-debate-playback.test.tsx | 34 +++++++++++++++++++ apps/web/core/debates/use-debate-playback.ts | 23 ++++++------- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/apps/web/core/debates/use-debate-playback.test.tsx b/apps/web/core/debates/use-debate-playback.test.tsx index 7dc637351e..4d3cd7ba05 100644 --- a/apps/web/core/debates/use-debate-playback.test.tsx +++ b/apps/web/core/debates/use-debate-playback.test.tsx @@ -1,4 +1,5 @@ import { act, renderHook, waitFor } from '@testing-library/react'; +import * as React from 'react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type { Debate } from './api'; @@ -104,6 +105,39 @@ describe('useDebatePlayback — playback URLs survive re-activation (GEO-2895)', await waitFor(() => expect(result.current.urls.slot1).not.toBeNull()); }); + + // StrictMode (on by default in dev) mounts, cleans up and re-runs every effect before the + // first fetch settles. The re-run must not be skipped as "already fetched", or the cancelled + // first run drops the URLs and the card sits on "Loading…" forever. + it('loads URLs under StrictMode', async () => { + const { result } = renderHook(() => useDebatePlayback(debateFixture(), true), { wrapper: React.StrictMode }); + + await waitFor(() => expect(result.current.urls.slot1).not.toBeNull()); + expect(result.current.urls.slot2).not.toBeNull(); + }); + + it('loads URLs when the card is scrolled away and back before the first fetch settles', async () => { + let settle: () => void = () => {}; + const gate = new Promise(resolve => { + settle = resolve; + }); + mocks.recordingUrl.mockImplementation(async ({ filename }: { filename: string }) => { + await gate; + return { url: `https://cdn.test/${filename}?sig=abc` }; + }); + + const debate = debateFixture(); + const { result, rerender } = renderHook(({ active }) => useDebatePlayback(debate, active), { + initialProps: { active: true }, + }); + + rerender({ active: false }); + rerender({ active: true }); + await act(async () => settle()); + + await waitFor(() => expect(result.current.urls.slot1).not.toBeNull()); + expect(result.current.urls.slot2).not.toBeNull(); + }); }); /** diff --git a/apps/web/core/debates/use-debate-playback.ts b/apps/web/core/debates/use-debate-playback.ts index d9a8366cbb..654838c564 100644 --- a/apps/web/core/debates/use-debate-playback.ts +++ b/apps/web/core/debates/use-debate-playback.ts @@ -155,6 +155,11 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { // // `useRecordingUrl` is a mutation rather than a query, so nothing upstream caches this — // every discarded URL is a real round trip. + // + // The key is claimed only once URLs are committed, never while a request is in flight. A + // cleanup that lands mid-flight (StrictMode's dev double-run, or scrolling away and back + // before the fetch settles) cancels that run, and a claim taken up front would make the + // re-run skip as "already fetched" — leaving the card on "Loading…" forever. const fetchedForRef = React.useRef(null); React.useEffect(() => { @@ -170,10 +175,7 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { if (fetchedForRef.current === recordingsKey) return; let cancelled = false; - fetchedForRef.current = recordingsKey; - const releaseKey = () => { - if (fetchedForRef.current === recordingsKey) fetchedForRef.current = null; - }; + fetchedForRef.current = null; setUrls({ slot1: null, slot2: null }); setError(null); @@ -182,17 +184,14 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { getRecordingPlaybackUrlRef.current({ debateId: debate.id, filename: slot2RecordingFilename }), ]) .then(([slot1Result, slot2Result]) => { - // Scrolled away mid-flight: nothing is committed, so release the key or the card - // would hold a claim on URLs it never received and never fetch again. - if (cancelled) { - releaseKey(); - return; - } + // Cancelled mid-flight: commit nothing and leave the key unclaimed so the next run + // fetches again. + if (cancelled) return; + fetchedForRef.current = recordingsKey; setUrls({ slot1: slot1Result.url, slot2: slot2Result.url }); }) .catch(caught => { - // Same on failure, otherwise one error leaves the card permanently on "Loading…". - releaseKey(); + // The key was never claimed, so the next activation retries. if (!cancelled) setError(caught instanceof Error ? caught.message : 'Could not load recordings.'); }); From 024883cb607c59d1a887eb5ae951e27ba8ec20cf Mon Sep 17 00:00:00 2001 From: o-p-o-p-o <204936872+o-p-o-p-o@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:53:55 +0100 Subject: [PATCH 2/5] feat(debates): persistent play/pause beside mute, click feedback, larger controls - Desktop: an always-visible play/pause control next to mute replaces the centred paused glyph. Clicking the video flashes a play/pause glyph briefly as feedback; it is not clickable. - Corner controls are 30% larger (32px -> 42px). - Mobile: the mute control no longer disappears after unmuting. It receded to hover-only, and touch has no hover. Co-Authored-By: Claude Opus 5 (1M context) --- .../debates/browse/debate-feed-player.tsx | 102 +++++++++++++----- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index 077215b8d0..9cd8582f93 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -17,7 +17,7 @@ import { Avatar } from '~/design-system/avatar'; import { RetrySmall } from '~/design-system/icons/retry-small'; import { Text } from '~/design-system/text'; -import { Play, Speaker, SpeakerMuted } from './icons'; +import { Pause, Play, Speaker, SpeakerMuted } from './icons'; import { WinnerVoteButton } from './winner-vote-button'; type DebateFeedPlayerProps = { @@ -98,6 +98,25 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb const showReplay = ready && playbackEnded && !hasVoted; const showPausedGlyph = ready && userPaused && !playbackEnded; + // Clicking the video briefly flashes the action it just took — feedback only, not a control. + const [flash, setFlash] = React.useState<{ icon: 'play' | 'pause'; visible: boolean }>({ + icon: 'play', + visible: false, + }); + const flashTimeoutRef = React.useRef | null>(null); + React.useEffect( + () => () => { + if (flashTimeoutRef.current) clearTimeout(flashTimeoutRef.current); + }, + [] + ); + const toggleFromVideo = () => { + setFlash({ icon: playing ? 'pause' : 'play', visible: true }); + if (flashTimeoutRef.current) clearTimeout(flashTimeoutRef.current); + flashTimeoutRef.current = setTimeout(() => setFlash(current => ({ ...current, visible: false })), 600); + togglePlayback(); + }; + return (
- - - ) : ready ? ( - // Feed debates autoplay muted, so the unmute control stays visible during - // playback — otherwise there's no way to hear audio. Once unmuted it recedes - // to hover-only. - { - measurement.control(mutedByUser ? 'unmute' : 'mute'); - setMutedByUser(current => !current); - }} - className={ - mutedByUser - ? undefined - : 'opacity-0 transition-opacity group-hover:opacity-100 focus-visible:opacity-100' - } - > - {mutedByUser ? : } - + ready ? ( +
+ {/* Desktop: a persistent play/pause beside the mute control. Mobile keeps the + centred paused glyph and tap-to-toggle instead. */} + + {playing ? : } + + {showReplay ? ( + + + + ) : ( + // Feed debates autoplay muted, so the unmute control stays visible during + // playback — otherwise there's no way to hear audio. Once unmuted it recedes + // to hover-only on desktop; touch has no hover, so on mobile it stays visible + // or there'd be no way to find it again. + { + measurement.control(mutedByUser ? 'unmute' : 'mute'); + setMutedByUser(current => !current); + }} + className={ + mutedByUser + ? undefined + : 'opacity-0 transition-opacity group-hover:opacity-100 focus-visible:opacity-100 md:opacity-100' + } + > + {mutedByUser ? : } + + )} +
) : null } /> @@ -146,7 +179,7 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb subtitle={activeSlot === 2 ? subtitle : null} mutedByUser={mutedByUser} onPlaybackTick={onPlaybackTick} - onToggle={togglePlayback} + onToggle={toggleFromVideo} votes={votes} scrubber={ ready ? ( @@ -172,17 +205,29 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb } /> + {/* Mobile only — desktop has the persistent play/pause beside the mute control. */} {showPausedGlyph && ( )} + {/* Desktop only — mobile already shows the centred paused glyph in this spot. */} +
+ {flash.icon === 'pause' ? : } +
+ {error && ( {error} @@ -339,7 +384,10 @@ function ControlCircle({ event.stopPropagation(); onClick(); }} - className={cx('grid size-8 place-items-center rounded-full bg-white text-text shadow-light', className)} + className={cx( + 'grid size-10.5 place-items-center rounded-full bg-white text-text shadow-light [&>svg]:scale-130', + className + )} > {children} From bdd6af0ef9d99d60eadf8987fc6478c96b4f2dd0 Mon Sep 17 00:00:00 2001 From: o-p-o-p-o <204936872+o-p-o-p-o@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:04:20 +0100 Subject: [PATCH 3/5] feat(debates): use the explore claims icon in the fullscreen interaction bar The fullscreen feed's Claims button (vertical rail on desktop, pill bar on mobile) drew InfoSmall, a question mark in a ring. Explore's debate card uses ExploreClaimsIcon. Use that in both places so the claims icon is the same everywhere. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/core/debates/browse/debate-interaction-bar.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/web/core/debates/browse/debate-interaction-bar.tsx b/apps/web/core/debates/browse/debate-interaction-bar.tsx index e60a2e1218..5b93cbdb7c 100644 --- a/apps/web/core/debates/browse/debate-interaction-bar.tsx +++ b/apps/web/core/debates/browse/debate-interaction-bar.tsx @@ -4,10 +4,10 @@ import * as React from 'react'; import cx from 'classnames'; -import { InfoSmall } from '~/design-system/icons/info-small'; import { Text } from '~/design-system/text'; import { EntityVoteButtons } from '~/partials/entity-page/entity-vote-buttons'; +import { ExploreClaimsIcon } from '~/partials/explore/explore-claims-icon'; import { Comment, Share } from './icons'; @@ -50,7 +50,7 @@ export function DebateInteractionBar({ presentation="debate-vertical" /> } ariaLabel="Comments" /> - } ariaLabel="Claims" /> + } ariaLabel="Claims" /> } ariaLabel="Share debate" expanded={shareOpen} />
); @@ -65,7 +65,7 @@ export function DebateInteractionBar({ presentation="debate-horizontal" /> } label={String(commentCount)} ariaLabel="Comments" /> - } label={String(claimsCount)} ariaLabel="Claims" /> + } label={String(claimsCount)} ariaLabel="Claims" /> } label="Share" ariaLabel="Share debate" expanded={shareOpen} /> ); From d77b0458cfa1845222be66f776f039a6be4112d9 Mon Sep 17 00:00:00 2001 From: Nate <204936872+o-p-o-p-o@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:06:28 +0100 Subject: [PATCH 4/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/web/core/debates/browse/debate-feed-player.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index 9cd8582f93..f35d692cbf 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -136,7 +136,7 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb {/* Desktop: a persistent play/pause beside the mute control. Mobile keeps the centred paused glyph and tap-to-toggle instead. */} From df81e5b414cee74d0741460daea5ce8fbee23ad6 Mon Sep 17 00:00:00 2001 From: Nate <204936872+o-p-o-p-o@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:06:36 +0100 Subject: [PATCH 5/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/web/core/debates/browse/debate-feed-player.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index f35d692cbf..61259d7d42 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -385,7 +385,7 @@ function ControlCircle({ onClick(); }} className={cx( - 'grid size-10.5 place-items-center rounded-full bg-white text-text shadow-light [&>svg]:scale-130', + 'grid size-10.5 place-items-center rounded-full bg-white text-text shadow-light [&>svg]:scale-[1.3]', className )} >