feat: add dynamic progressInterval support #7
Conversation
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant YoutubePlayer
participant WebView/PlayerHTML
participant YoutubeIframeScript
App->>YoutubePlayer: Render with progressInterval (e.g., 1000)
YoutubePlayer->>WebView/PlayerHTML: sendCommand('updateProgressInterval', value)
WebView/PlayerHTML->>YoutubeIframeScript: window.playerCommands.updateProgressInterval(value)
YoutubeIframeScript->>YoutubeIframeScript: Set interval, start/stop progress tracking
YoutubeIframeScript-->>WebView/PlayerHTML: onProgress callback at specified interval
WebView/PlayerHTML-->>YoutubePlayer: onProgress event
YoutubePlayer-->>App: onProgress callback invoked
App->>YoutubePlayer: Update progressInterval (e.g., toggle to 0)
YoutubePlayer->>WebView/PlayerHTML: sendCommand('updateProgressInterval', newValue)
WebView/PlayerHTML->>YoutubeIframeScript: Update or clear interval, stop/start tracking
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying react-native-youtube-bridge with
|
| Latest commit: |
6a57e46
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://574d504b.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://feat-interval-option.react-native-youtube-bridge.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/types/message.ts (1)
13-36: Consider derivingCommandTypeautomatically fromplayerCommandsfor maintainabilityKeeping this hard-coded string union in sync with the actual implementation (
window.playerCommandson the iframe side and any façade on the React side) is error-prone.
A small typo or a new command added in one place but not the other will escape compile-time checks.Typical pattern:
export const playerCommands = { play() { /* … */ }, pause() { /* … */ }, // … } as const; export type CommandType = keyof typeof playerCommands;This removes duplication and guarantees type safety at every call-site.
src/hooks/youtubeIframeScripts.ts (1)
34-35: Explicitly cast interval to number forsetIntervalEven after the guard, cast to make TS/ESLint happy and avoid surprises if the value was boxed (
new Number(1000)):- }, window.currentInterval); + }, Number(window.currentInterval));src/types/youtube.ts (1)
50-53: Clarify edge-case:onProgresssupplied while tracking disabledDocs say the callback is ignored if
progressIntervalis falsy.
It might be worth emitting a development-mode warning to help consumers noticing that their handler is never called.src/YoutubePlayer.web.tsx (1)
13-14: Web path is robust; minor typings / nit-level notes✔️ Interval is ignored when falsy (
0,undefined), matching expected “disabled” semantics – good.Nitpicks
•progressIntervalref is initialised from the prop, but the prop is namedintervallocally; consider keeping the same name to avoid mental mapping.
•progressIntervalis declared asNodeJS.Timeout; in the browser this is actuallynumber. UsingReturnType<typeof setInterval>avoids the mismatch.No functional blockers here.
Also applies to: 43-44, 53-56, 83-84, 239-248
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README-ko_kr.md(2 hunks)README.md(2 hunks)example/src/App.tsx(3 hunks)src/YoutubePlayer.tsx(5 hunks)src/YoutubePlayer.web.tsx(5 hunks)src/hooks/useCreateLocalPlayerHtml.ts(1 hunks)src/hooks/youtubeIframeScripts.ts(2 hunks)src/types/message.ts(1 hunks)src/types/youtube.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/YoutubePlayer.tsx (1)
src/types/message.ts (1)
CommandType(13-36)
🔇 Additional comments (4)
README-ko_kr.md (1)
215-230: Documentation update looks goodThe Korean README correctly describes the new behaviour and gives a concrete example. 👍
README.md (1)
215-230: Docs are consistent with implementationThe English README mirrors the code changes and provides a clear example. Thanks for keeping the docs up-to-date.
example/src/App.tsx (1)
29-30: ```bash
#!/bin/bashLocate the file that defines the injected iframe scripts
echo "=== Searching for youtubeIframeScripts definition ==="
SCRIPT_FILE=$(rg -l "youtubeIframeScripts" -g "*.ts" . | grep -v "useCreateLocalPlayerHtml.ts" | head -n1)
echo "Found script file: $SCRIPT_FILE"echo
echo "=== Inspecting updateProgressInterval implementation in snippet ==="
rg -n -B5 -A20 "updateProgressInterval" "$SCRIPT_FILE"echo
echo "=== Looking for setInterval calls in snippet ==="
rg -n -C3 "setInterval" "$SCRIPT_FILE"</details> <details> <summary>src/YoutubePlayer.tsx (1)</summary> `230-235`: **Guard `updateProgressInterval` call against `undefined`** On first render `progressInterval` is often `undefined`. Sending that down re-enables the bug fixed above. Protect the call: ```diff -useEffect(() => { - if (isReady) { - sendCommand('updateProgressInterval', [progressInterval]); - } -}, [progressInterval, isReady, sendCommand]); +useEffect(() => { + if (isReady && progressInterval !== undefined) { + sendCommand('updateProgressInterval', [progressInterval]); + } +}, [progressInterval, isReady, sendCommand]);[ suggest_essential_refactor ]
Summary by CodeRabbit
New Features
Documentation