Skip to content

Commit f14cfa4

Browse files
fix: offline video preferences after ClientOnly hydration (#576)
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent d77c1df commit f14cfa4

3 files changed

Lines changed: 155 additions & 14 deletions

File tree

packages/workshop-app/app/components/epic-video.tsx

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { useIsOnline } from '#app/utils/online.ts'
3333
import { useRootLoaderData } from '#app/utils/root-loader.ts'
3434
import { Icon } from './icons.tsx'
3535
import { Loading } from './loading.tsx'
36+
import { useApplyNativeVideoPreferences } from './native-video-preferences.ts'
3637
import { OfflineVideoActionButtons } from './offline-video-actions.tsx'
3738
import { useOptionalUser } from './user.tsx'
3839
import { useWorkshopConfig } from './workshop-config.tsx'
@@ -465,6 +466,8 @@ function EpicVideo({
465466
}) {
466467
const muxPlayerRef = React.useRef<MuxPlayerRefAttributes>(null)
467468
const nativeVideoRef = React.useRef<HTMLVideoElement>(null)
469+
const [nativeVideoElement, setNativeVideoElement] =
470+
React.useState<HTMLVideoElement | null>(null)
468471
const isOnline = useIsOnline()
469472
const playerPreferences = usePlayerPreferences()
470473
const rootData = useRootLoaderData()
@@ -539,6 +542,13 @@ function EpicVideo({
539542
const [offlineStartTime, setOfflineStartTime] = React.useState<number | null>(
540543
null,
541544
)
545+
const setNativeVideoRef = React.useCallback(
546+
(element: HTMLVideoElement | null) => {
547+
nativeVideoRef.current = element
548+
setNativeVideoElement(element)
549+
},
550+
[],
551+
)
542552
const timestampRegex = /(\d+:\d+)/g
543553
// turn the transcript into an array of React elements
544554
const transcriptElements: Array<React.ReactNode> = []
@@ -596,19 +606,11 @@ function EpicVideo({
596606
)
597607
}
598608

599-
React.useEffect(() => {
600-
if (!nativeVideoRef.current) return
601-
if (typeof playerPreferences?.playbackRate === 'number') {
602-
nativeVideoRef.current.playbackRate = playerPreferences.playbackRate
603-
}
604-
if (typeof playerPreferences?.volumeRate === 'number') {
605-
nativeVideoRef.current.volume = playerPreferences.volumeRate
606-
}
607-
}, [
608-
playerPreferences?.playbackRate,
609-
playerPreferences?.volumeRate,
610-
shouldUseOfflineVideo,
611-
])
609+
useApplyNativeVideoPreferences({
610+
shouldApply: shouldUseOfflineVideo,
611+
videoElement: nativeVideoElement,
612+
playerPreferences,
613+
})
612614

613615
React.useEffect(() => {
614616
if (!shouldUseOfflineVideo) return
@@ -801,7 +803,7 @@ function EpicVideo({
801803
}
802804
>
803805
<video
804-
ref={nativeVideoRef}
806+
ref={setNativeVideoRef}
805807
slot="media"
806808
aria-label={title}
807809
className="h-full w-full"
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as React from 'react'
2+
3+
type NativeVideoPlayerPreferences =
4+
| {
5+
playbackRate?: number | null
6+
volumeRate?: number | null
7+
}
8+
| null
9+
| undefined
10+
11+
export function useApplyNativeVideoPreferences({
12+
shouldApply,
13+
videoElement,
14+
playerPreferences,
15+
}: {
16+
shouldApply: boolean
17+
videoElement: HTMLVideoElement | null
18+
playerPreferences: NativeVideoPlayerPreferences
19+
}) {
20+
React.useEffect(() => {
21+
if (!shouldApply || !videoElement) return
22+
23+
if (typeof playerPreferences?.playbackRate === 'number') {
24+
videoElement.playbackRate = playerPreferences.playbackRate
25+
}
26+
27+
if (typeof playerPreferences?.volumeRate === 'number') {
28+
videoElement.volume = playerPreferences.volumeRate
29+
}
30+
}, [
31+
playerPreferences?.playbackRate,
32+
playerPreferences?.volumeRate,
33+
shouldApply,
34+
videoElement,
35+
])
36+
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import * as React from 'react'
2+
import { expect, test } from 'vitest'
3+
import { page } from 'vitest/browser'
4+
import { render } from 'vitest-browser-react'
5+
import { useApplyNativeVideoPreferences } from '#app/components/native-video-preferences.ts'
6+
7+
function LegacyHydrationDelayedVideoPreferences() {
8+
const playbackRate = 1.75
9+
const volumeRate = 0.25
10+
const shouldUseOfflineVideo = true
11+
const nativeVideoRef = React.useRef<HTMLVideoElement>(null)
12+
const [isHydrated, setIsHydrated] = React.useState(false)
13+
const [snapshot, setSnapshot] = React.useState('legacy:pending')
14+
15+
React.useEffect(() => {
16+
setIsHydrated(true)
17+
}, [])
18+
19+
React.useEffect(() => {
20+
if (!nativeVideoRef.current) return
21+
if (typeof playbackRate === 'number') {
22+
nativeVideoRef.current.playbackRate = playbackRate
23+
}
24+
if (typeof volumeRate === 'number') {
25+
nativeVideoRef.current.volume = volumeRate
26+
}
27+
}, [playbackRate, volumeRate, shouldUseOfflineVideo])
28+
29+
React.useEffect(() => {
30+
if (!isHydrated || !nativeVideoRef.current) return
31+
setSnapshot(
32+
`legacy:${nativeVideoRef.current.playbackRate.toFixed(2)}|${nativeVideoRef.current.volume.toFixed(2)}`,
33+
)
34+
}, [isHydrated])
35+
36+
return (
37+
<div>
38+
{isHydrated ? (
39+
<video ref={nativeVideoRef} aria-label="legacy-video" />
40+
) : (
41+
<span>legacy-fallback</span>
42+
)}
43+
<span>{snapshot}</span>
44+
</div>
45+
)
46+
}
47+
48+
function FixedHydrationDelayedVideoPreferences() {
49+
const playerPreferences = { playbackRate: 1.75, volumeRate: 0.25 }
50+
const shouldUseOfflineVideo = true
51+
const nativeVideoRef = React.useRef<HTMLVideoElement>(null)
52+
const [nativeVideoElement, setNativeVideoElement] =
53+
React.useState<HTMLVideoElement | null>(null)
54+
const [isHydrated, setIsHydrated] = React.useState(false)
55+
const [snapshot, setSnapshot] = React.useState('fixed:pending')
56+
const setNativeVideoRef = React.useCallback(
57+
(element: HTMLVideoElement | null) => {
58+
nativeVideoRef.current = element
59+
setNativeVideoElement(element)
60+
},
61+
[],
62+
)
63+
64+
useApplyNativeVideoPreferences({
65+
shouldApply: shouldUseOfflineVideo,
66+
videoElement: nativeVideoElement,
67+
playerPreferences,
68+
})
69+
70+
React.useEffect(() => {
71+
setIsHydrated(true)
72+
}, [])
73+
74+
React.useEffect(() => {
75+
if (!nativeVideoElement || !nativeVideoRef.current) return
76+
setSnapshot(
77+
`fixed:${nativeVideoRef.current.playbackRate.toFixed(2)}|${nativeVideoRef.current.volume.toFixed(2)}`,
78+
)
79+
}, [nativeVideoElement])
80+
81+
return (
82+
<div>
83+
{isHydrated ? (
84+
<video ref={setNativeVideoRef} aria-label="fixed-video" />
85+
) : (
86+
<span>fixed-fallback</span>
87+
)}
88+
<span>{snapshot}</span>
89+
</div>
90+
)
91+
}
92+
93+
test('legacy ref-only effect misses preferences after hydration (aha)', async () => {
94+
await render(<LegacyHydrationDelayedVideoPreferences />)
95+
96+
await expect.element(page.getByText('legacy:1.00|1.00')).toBeVisible()
97+
})
98+
99+
test('applies preferences when delayed video element mounts', async () => {
100+
await render(<FixedHydrationDelayedVideoPreferences />)
101+
102+
await expect.element(page.getByText('fixed:1.75|0.25')).toBeVisible()
103+
})

0 commit comments

Comments
 (0)