feat(layouts): overridable participant mirroring#2106
Conversation
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration Context
participant Layout as Layout Component
participant ParticipantView as ParticipantView
participant VideoRenderer as VideoRenderer
Config->>Layout: Provide forceMirrorParticipants config
Layout->>ParticipantView: Pass mirror prop (from config)
ParticipantView->>VideoRenderer: Pass mirror prop
VideoRenderer->>VideoRenderer: Compute RTCView.mirror<br/>(prefer mirrorOverride if defined)
VideoRenderer->>VideoRenderer: Apply mirror setting to video
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-native-sdk/src/components/Call/CallParticipantsList/CallParticipantsList.tsx (1)
205-228:⚠️ Potential issue | 🟡 MinorMissing
mirrorinrenderItemdependency array.The
mirrorprop is used insiderenderItem(line 219) but is not included in the dependency array (line 227). Ifmirrorchanges after initial render, the callback will use a stale value. Whilemirroris likely static from configuration in practice, consider adding it to the dependency array for correctness, or document why it's intentionally excluded.🔧 Suggested fix
const renderItem = useCallback<NonNullable<FlatListProps['renderItem']>>( ({ item: participant }) => { const isVisible = viewableParticipantSessionIds.current.has( participant.sessionId, ); return ( <> {ParticipantView && ( <ParticipantView participant={participant} style={itemContainerStyle} trackType="videoTrack" isVisible={isVisible} supportedReactions={supportedReactions} mirror={mirror} {...participantProps} /> )} </> ); }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [itemContainerStyle], + // eslint-disable-next-line react-hooks/exhaustive-deps -- participantProps intentionally excluded to avoid re-renders + [itemContainerStyle, mirror, supportedReactions, ParticipantView], );
🤖 Fix all issues with AI agents
In
`@packages/react-native-sdk/src/components/Participant/ParticipantView/VideoRenderer/index.tsx`:
- Around line 114-117: The mirroring logic in the const mirror calculation lets
mirrorOverride=true force mirroring even for screen shares; update the
expression in ParticipantView's VideoRenderer (the const named mirror that reads
mirrorOverride, isLocalParticipant, isScreenSharing, and direction) so that if
isScreenSharing is true it always results in false (no mirror), otherwise
preserve the existing behavior (use mirrorOverride when provided, else mirror
only for local camera with direction === 'front'). Ensure the check
short-circuits screen-share first before applying mirrorOverride.
💡 Overview
Allows integrators to control the video mirroring for any participant on the call.
🎫 Ticket: https://linear.app/stream/issue/RN-341/overridable-participant-mirroring
📑 Docs: https://github.com/GetStream/docs-content/pull/969
Summary by CodeRabbit
Release Notes