feat: add muteChange event with subscription-based muted state tracking - #85
Merged
Conversation
🦋 Changeset detectedLatest commit: f877593 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User/UI
participant Iframe as YouTube Iframe Player
participant Core as WebYoutubePlayerController
participant Bridge as Message Bridge (WebView)
participant Hook as useYouTubeEvent Hook
participant App as React Component
Note over Hook: App subscribes to 'muteChange'
Hook->>Bridge: INTERNAL_SET_MUTE_TRACKING(true)
Bridge->>Core: setMutedTrackingEnabled(true)
Core->>Core: Start polling (250ms)
UI->>Iframe: click mute/unmute or call player.mute()/unMute()
Iframe->>Core: onMuteChange(muted)
Core->>Bridge: emit message { type: 'muteChange', muted }
Bridge->>Hook: deliver 'muteChange' event
Hook->>App: update isMuted state
Note over Core: Polling: syncMutedStateFromPlayer()
Core->>Iframe: query muted state
Iframe-->>Core: current muted
alt changed
Core->>Bridge: emit 'muteChange'
Bridge->>Hook: deliver event
Hook->>App: update state
end
App->>Hook: unsubscribe 'muteChange'
Hook->>Bridge: INTERNAL_SET_MUTE_TRACKING(false)
Bridge->>Core: setMutedTrackingEnabled(false)
Core->>Core: Stop polling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Deploying react-native-youtube-bridge-example with
|
| Latest commit: |
f877593
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9a2587ee.react-native-youtube-bridge-example.pages.dev |
| Branch Preview URL: | https://feat-mute-change-event-sync.react-native-youtube-bridge-example.pages.dev |
Deploying react-native-youtube-bridge with
|
| Latest commit: |
40be6b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a2b8bbf6.react-native-youtube-bridge.pages.dev |
| Branch Preview URL: | https://feat-mute-change-event-sync.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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/WebYoutubePlayerController.ts (1)
122-138:⚠️ Potential issue | 🟡 MinorUse the normalized ready value in the
onReadypayload too.
readyMutedalready falls back tothis.desiredMuted, but Line 130 still forwardsplayerInfo.muted. If that fallback path is hit,onReadyandmuteChangewill disagree about the initial muted state.Proposed fix
this.callbacks.onReady?.({ availablePlaybackRates: playerInfo.availablePlaybackRates, availableQualityLevels: playerInfo.availableQualityLevels, currentTime: playerInfo.currentTime, duration: playerInfo.duration, - muted: playerInfo.muted, + muted: readyMuted, playbackQuality: playerInfo.playbackQuality, playbackRate: playerInfo.playbackRate, playerState: playerInfo.playerState, size: playerInfo.size, volume: playerInfo.volume, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/WebYoutubePlayerController.ts` around lines 122 - 138, The onReady payload is still using playerInfo.muted instead of the normalized readyMuted, which can cause a mismatch between the initial onReady state and the later update via updateMutedState; change the muted field sent to this.callbacks.onReady in WebYoutubePlayerController (the onReady call that currently references playerInfo.muted) to use the computed readyMuted (which falls back to this.desiredMuted) so the initial event and the updateMutedState(readyMuted, true) call agree.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example/src/App.tsx`:
- Line 44: The default value for the isMuted state from useYouTubeEvent does not
match the player's initial config (player initialized with muted: true), causing
a brief UI inconsistency; update the call to useYouTubeEvent(player,
'muteChange', true) so isMuted starts as true (referencing isMuted,
useYouTubeEvent, and the player initialization with muted: true) ensuring the
mute button reflects the real initial state until the first muteChange event
arrives.
In `@packages/core/src/WebYoutubePlayerController.ts`:
- Around line 297-306: The mute() and unMute() methods set desiredMuted and then
call updateMutedState even when no player exists, causing onMuteChange to fire
before createPlayer() attaches a player; change both methods (mute() and
unMute()) to set this.desiredMuted as before but bail out early if this.player
is null (i.e., only call this.player.mute()/unMute() and
this.updateMutedState(...) when this.player is present) so the event is emitted
only after an actual player is attached.
In `@packages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.ts`:
- Around line 263-269: The mute/unMute handlers call updateMutedState before
checking for a real player, causing a muteChange to emit even when the
iframe/player isn't ready; adjust mute and unMute so they still record the
desired muted value (keep desiredMuted or the equivalent setter) immediately but
only call updateMutedState(...) and emit the change after confirming player
exists (i.e., only when player is truthy), then invoke
player.mute()/player.unMute(); reference the mute, unMute, player,
updateMutedState, and desiredMuted symbols when making the change.
---
Outside diff comments:
In `@packages/core/src/WebYoutubePlayerController.ts`:
- Around line 122-138: The onReady payload is still using playerInfo.muted
instead of the normalized readyMuted, which can cause a mismatch between the
initial onReady state and the later update via updateMutedState; change the
muted field sent to this.callbacks.onReady in WebYoutubePlayerController (the
onReady call that currently references playerInfo.muted) to use the computed
readyMuted (which falls back to this.desiredMuted) so the initial event and the
updateMutedState(readyMuted, true) call agree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 154ff4b9-5ef4-47e4-a9c7-a7826e13bc7e
📒 Files selected for processing (19)
.changeset/flat-bats-greet.mdREADME-ko_kr.mdREADME.mdexample/src/App.tsxpackages/core/src/WebYoutubePlayerController.tspackages/core/src/types/index.tspackages/core/src/types/webview.tspackages/react-native-youtube-bridge/README-ko_kr.mdpackages/react-native-youtube-bridge/README.mdpackages/react-native-youtube-bridge/src/YoutubeView.tsxpackages/react-native-youtube-bridge/src/YoutubeView.web.tsxpackages/react-native-youtube-bridge/src/hooks/useCreateLocalPlayerHtml.tspackages/react-native-youtube-bridge/src/hooks/useYouTubeEvent.tspackages/react-native-youtube-bridge/src/hooks/youtubeIframeScripts.tspackages/react-native-youtube-bridge/src/modules/WebviewYoutubePlayerController.tspackages/react-native-youtube-bridge/src/modules/YoutubePlayer.tspackages/react-native-youtube-bridge/src/types/message.tspackages/web/src/YoutubePlayer.tsxpackages/web/src/types/message.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
muteChangeevent for real-time muted state updates.muteChangethrough core/web/webview/react-native bridge layers.muteChangeis subscribed (performance optimization).useYouTubeEvent(player, 'muteChange', false)for muted state.muteChangeusage and tracking behavior.Summary by CodeRabbit
New Features
muteChangeevent to receive real-time muted state updates (UI button and programmatic mute/unMute).useYouTubeEventsupports both state-based (e.g., isMuted via useYouTubeEvent(player, 'muteChange', false)) and callback-based subscriptions.Behavior
muteChangeis subscribed to (performance optimization).Documentation
muteChangeusage and migration to event-driven mute state.