VIDSOL-619: Mirror Self View Toggle#390
Conversation
02fd3f4 to
fe0648f
Compare
fe0648f to
e1d35ce
Compare
There was a problem hiding this comment.
Pull request overview
Adds a persisted Mirror Self View user preference and exposes it in both the waiting room (icon button) and the meeting room device settings menu, updating publisher/preview video styling accordingly.
Changes:
- Added a new
localStoragekey +UserContextstate formirrorSelfView(defaulting to mirrored). - Introduced UI controls to toggle the setting in the waiting room and in the camera device dropdown.
- Updated publisher/preview video containers to apply mirroring based on the new setting, plus i18n and test updates.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/utils/storage.ts | Adds MIRROR_SELF_VIEW storage key. |
| frontend/src/Context/user.tsx | Loads/persists mirrorSelfView into UserContext default settings. |
| frontend/src/components/WaitingRoom/VideoContainer/VideoContainer.tsx | Applies dynamic mirroring/objectFit and adds waiting room toggle button. |
| frontend/src/components/WaitingRoom/MirrorSelfViewButton/index.tsx | Barrel export for the new waiting room toggle button. |
| frontend/src/components/WaitingRoom/MirrorSelfViewButton/MirrorSelfViewButton.tsx | Implements waiting room mirror toggle + persistence. |
| frontend/src/components/Publisher/Publisher.tsx | Applies dynamic mirroring to in-call publisher rendering. |
| frontend/src/components/BackgroundEffects/BackgroundVideoContainer/BackgroundVideoContainer.tsx | Applies dynamic mirroring to background preview video element. |
| frontend/src/components/MeetingRoom/MirrorSelfViewToggle/MirrorSelfViewToggle.tsx | Implements meeting room dropdown toggle + persistence. |
| frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.tsx | Renders the new mirror toggle in the video settings dropdown. |
| frontend/src/locales/en.json | Adds i18n label for “Mirror Self View”. |
| frontend/src/locales/es.json | Adds i18n label for “Mirror Self View”. |
| frontend/src/locales/es-MX.json | Adds i18n label for “Mirror Self View”. |
| frontend/src/locales/it.json | Adds i18n label for “Mirror Self View”. |
| frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx | Updates tests to account for mirror toggle (and new provider setup). |
| frontend/src/components/BackgroundEffects/BackgroundVideoContainer/BackgroundVideoContainer.spec.tsx | Wraps tests with UserProvider due to new useUserContext dependency. |
| frontend/src/Context/tests/user.spec.tsx | Updates default settings expectations with mirrorSelfView. |
| frontend/src/Context/PublisherProvider/usePublisherQuality/usePublisherQuality.spec.tsx | Updates mocked UserContext default settings with mirrorSelfView. |
You can also share your feedback on Copilot code review. Take the survey.
frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/WaitingRoom/VideoContainer/VideoContainer.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/BackgroundEffects/BackgroundVideoContainer/BackgroundVideoContainer.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/BackgroundEffects/BackgroundVideoContainer/BackgroundVideoContainer.tsx
Show resolved
Hide resolved
|
Hi @johnny-quesada-developer @OscarFava 👋 This PR is ready for your first review. All Copilot review comments have been addressed (fixed Would love your eyes on it when you have a chance! 🙏 |
OscarFava
left a comment
There was a problem hiding this comment.
Does it have sense to have a button to mirror? I think it should be mirrored by default and we can allow users to change it with the environment variables.
| </> | ||
| )} | ||
| <DropdownSeparator /> | ||
| <MenuList sx={{ display: 'flex', flexDirection: 'column', mt: 1 }}> |
There was a problem hiding this comment.
Could you use tailwind? Please, ensure that it is visually correct, sometimes AI does not translate MUI to tailwind correctly.
frontend/src/components/MeetingRoom/MirrorSelfViewToggle/MirrorSelfViewToggle.tsx
Outdated
Show resolved
Hide resolved
| }; | ||
|
|
||
| return ( | ||
| <Box |
There was a problem hiding this comment.
Same here, tailwind and remember to use our tokens, not default colors.
There was a problem hiding this comment.
this should be addressed
| if (!publisherVideoElement) return; | ||
|
|
||
| // eslint-disable-next-line react-hooks/immutability | ||
| publisherVideoElement.style.objectFit = 'cover'; |
There was a problem hiding this comment.
We set object-fit: cover because the SDK injects the preview video with object-fit: contain, which leaves letterboxing
inside our 16:9 tile. Cover fills the tile consistently; the inline set is necessary because the SDK owns the element.
Can remove that if you do not think it is appropriate
| * Follows the same pattern as the Advanced Noise Suppression toggle in the audio menu. | ||
| * @returns {ReactElement} The MirrorSelfViewToggle component. | ||
| */ | ||
| const MirrorSelfViewToggle = (): ReactElement => { |
There was a problem hiding this comment.
Does it have sense to have a button to mirror? I think it should be mirrored by default and we can allow users to change it with the environment variables.
|
Hi @OscarFava, thanks for the question! 🙏 The feature actually has a user-first design rationale:
This is consistent with how mainstream video apps work (Teams, Zoom, Meet) — all expose a per-session mirror toggle. An env var would only work for operators deploying the app, not for end-users who want to flip it mid-call. It would also require a page reload to take effect, which is disruptive. Happy to discuss further — but I think the toggle is the right call here. What do you think? |
|
Hi @czoli1976, I see that this changes the UI. We should first align with the when a PR touches the UI in a non trivial way. |
|
Hi @VZaphod, thanks for flagging this! 🙏 Totally agree — happy to pause and align before proceeding. Could you point me to the right process or channel for UI alignment? I want to make sure the right people have visibility before this moves forward. |
|
@johnny-quesada-developer Friendly ping 👋 — this PR has been open for 16 days and is waiting for your initial review. CI is green, all Copilot bot feedback addressed. Happy to walk through the implementation if helpful! 🙏 |
Adds a user-toggleable mirror (horizontal flip) for the self-view video. Previously, the self-view was always mirrored via scaleX(-1). Now users can toggle mirroring on/off, and the preference persists in localStorage. The toggle is available in two places: - Waiting Room: a badge-style Flip icon button in the upper-right corner of the video tile (aligned with the Background Effects badge) - Conference Room: a toggle menu item in the camera device dropdown, always visible on all browsers (not gated by media processor support), following the same pattern as Advanced Noise Suppression in the audio menu Key design decisions: - Default: mirrored (true) — preserves existing behaviour - Inverted transform logic: SDK mirrors via .OT_mirrored; when mirror is ON we set transform:none, when OFF we apply scaleX(-1) to cancel it - objectFit changed from contain to cover across all three video components - Preference stored as localStorage key mirrorSelfView - i18n keys added for en, es, es-MX, it locales - Fixed pre-existing no-floating-promises lint error in useSuspenseUntilAppConfigReady Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add mirrorSelfView: true to user.spec.tsx expected defaultSettings - Add UserProvider wrapper to BackgroundVideoContainer.spec.tsx (component now uses useUserContext) - Update DeviceSettingsMenu.spec.tsx: add providers.user and userContext to RenderOptions, use queryAllByTestId for dropdown-separator (two separators now rendered) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove providers.appConfig (does not exist in current codebase) - Use providers.user only for MirrorSelfViewToggle context - Restore env.partialUpdate for ALLOW_BACKGROUND_EFFECTS test - Remove appConfigContext from RenderOptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ainer - Replace transform:'none' with removeProperty/'' so SDK's .OT_mirrored CSS (scale(-1,1) on .OT_video-element) is not inadvertently overridden when mirror is ON - Use scaleX(1) when mirror is OFF to correctly cancel SDK's scale(-1,1) - Remove unused isSMViewport hook and dep from BackgroundVideoContainer effect Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…line The react-hooks/immutability rule fires on direct property assignments (`element.style.transform = 'scaleX(1)'`) but not on method calls (`removeProperty`) or ternary assignments. The disable comments were placed on the wrong lines — moved to sit immediately above the assignment that actually triggers the rule, and removed the now-unused disable in BackgroundVideoContainer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1dcb6e3 to
1ec7de8
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3927b7b to
b6d6b23
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hello, @czoli1976 We are still pending UI review as I DO NOT approve this PR design (Does not seem to match with UI previous figma). So, please, do not ping the team to review before solving the design. Thanks. |
|
vcr:deploy |
❌ Deployment to VCR FailedCommit: |
|
vcr:deploy |
❌ Deployment to VCR FailedCommit: |
|
vcr:deploy |
❌ Deployment to VCR FailedCommit: |
|
Closing in favor of #414 (same branch pushed to upstream to enable VCR deploy) |
Hi team! 👋
Resolves VIDSOL-619
Checklist
develop(notmain).Known Issue.docs/KNOWN_ISSUES.md?Issues.What is this PR doing?
Adds a Mirror Self View toggle so users can choose whether their preview is mirrored. Default is ON (mirrored) and
persisted in localStorage.
Where it appears:
Mirror behavior:
scaleX(-1) inline.
Styling refactor:
Manual test notes remain the same; ensure toggles update immediately and persist on refresh across browsers.
Mirror logic:
mirrorSelfViewtrue(default)none— the SDK's.OT_mirroredclass handles the flipfalsescaleX(-1)— cancels the SDK mirrorChanges:
storage.ts— AddedMIRROR_SELF_VIEWkeyuser.tsx— AddedmirrorSelfView: booleantoUserContext/UserProvider, loaded fromlocalStorageMirrorSelfViewButton.tsx(new) — Waiting room icon buttonMirrorSelfViewToggle.tsx(new) — Conference room toggle menu itemVideoContainer.tsx— Replaced static Tailwindchild:-scale-x-100with reactiveuseEffect; addedMirrorSelfViewButtonPublisher.tsx— Dynamic transform viauseEffect;objectFit: coverBackgroundVideoContainer.tsx— Dynamic transform +objectFit: coveren.json,es.json,es-MX.json,it.json— Addeddevices.video.mirrorSelfViewi18n keyuser.spec.tsx,BackgroundVideoContainer.spec.tsx,DeviceSettingsMenu.spec.tsx— Updated testsHow should this be manually tested?
Waiting room:
Conference room:
Tested on macOS Tahoe 26.3 (arm64):
Instructions for Testing
Setup:
22.
Waiting room:
- ON → video mirrored. OFF → unmirrored.
Meeting room:
- ON → mirrored; OFF → unmirrored.
Fallback:
Cross-browser:
Persistence:
Regression: