Skip to content

Conversation

@arnaud-lebreton-rofim
Copy link
Contributor

@arnaud-lebreton-rofim arnaud-lebreton-rofim commented Sep 17, 2025

What is this PR doing?

Persist, in localStorage, an user action to enable/disable his mic or camera upon page refresh when on a meeting.
Reset those value upon re-entering the WaitingRoom.

How should this be manually tested?

On the WaitingRoom, disable mic or camera
Join the Room, mic or camera is disabled
Refresh the page, mic or camera stay disabled

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDSOL-269

Checklist

[x] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[x] Resolves an item reported in Issues.
If yes, which issue? 215

@VZaphod VZaphod requested review from OscarFava and Copilot October 24, 2025 12:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements localStorage persistence for user audio/video device preferences, ensuring that when a user disables their microphone or camera during a meeting, these preferences are maintained across page refreshes until they return to the waiting room.

Key Changes:

  • Added localStorage keys for tracking audio and video enabled states
  • Modified publisher options to read from localStorage when initializing audio/video states
  • Reset device preferences to enabled when entering the waiting room

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/utils/storage.ts Added new storage keys for audio and video enabled states
frontend/src/Context/PublisherProvider/usePublisherOptions/usePublisherOptions.tsx Modified to check localStorage when setting publishAudio and publishVideo properties
frontend/src/Context/PublisherProvider/usePublisherOptions/usePublisherOptions.spec.tsx Added test coverage for localStorage-based audio/video disable functionality
frontend/src/Context/PublisherProvider/usePublisher/usePublisher.tsx Updated toggle functions to persist audio/video states to localStorage
frontend/src/Context/PreviewPublisherProvider/usePreviewPublisher/usePreviewPublisher.tsx Added localStorage reset logic in waiting room and persistence in toggle functions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@VZaphod VZaphod changed the title Persist device preference in localStorage VIDSOL-269: Persist device preference in localStorage Oct 24, 2025
@VZaphod
Copy link
Contributor

VZaphod commented Oct 24, 2025

Hi @arnaud-lebreton-rofim, could you check the comments from copilot?
Thanks for the PR!

@arnaud-lebreton-rofim
Copy link
Contributor Author

@VZaphod i'll handle this asap 👍

@arnaud-lebreton-rofim arnaud-lebreton-rofim force-pushed the arnaudlebreton/persit-device-preferences branch from 91e91e3 to fe3fa51 Compare October 27, 2025 12:37
@arnaud-lebreton-rofim
Copy link
Contributor Author

@VZaphod i simplified the code a bit according to Copilot suggestion.
please let me now if you think this PR needs more work.

@arnaud-lebreton-rofim arnaud-lebreton-rofim force-pushed the arnaudlebreton/persit-device-preferences branch from fe3fa51 to 080a058 Compare October 28, 2025 08:40
@VZaphod
Copy link
Contributor

VZaphod commented Oct 28, 2025

Thanks @arnaud-lebreton-rofim, the team will check the PR when they get some bandwidth.
Thanks!!

Copy link
Contributor

@OscarFava OscarFava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! Just a few proposed improvements! Thanks for the great work!

@arnaud-lebreton-rofim
Copy link
Contributor Author

@OscarFava
I can't do that : setStorageItem(STORAGE_KEYS.VIDEO_SOURCE_ENABLED, isVideoEnabled);
"setStorageItem" expect a string as second argument.

@OscarFava
Copy link
Contributor

@OscarFava I can't do that : setStorageItem(STORAGE_KEYS.VIDEO_SOURCE_ENABLED, isVideoEnabled); "setStorageItem" expect a string as second argument.

You are right, we are sorry, you can use .toString() to simplify the solution? Thanks a lot! cc @johnny-quesada-developer @VZaphod

@arnaud-lebreton-rofim arnaud-lebreton-rofim force-pushed the arnaudlebreton/persit-device-preferences branch 2 times, most recently from a3c4b5d to b934799 Compare November 13, 2025 09:13
@arnaud-lebreton-rofim arnaud-lebreton-rofim force-pushed the arnaudlebreton/persit-device-preferences branch from b934799 to 2ebbdf0 Compare November 13, 2025 09:42
@arnaud-lebreton-rofim
Copy link
Contributor Author

@OscarFava thx for your review
I've made the changes accordingly

Copy link
Contributor

@OscarFava OscarFava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnny-quesada-developer johnny-quesada-developer added the update-screenshots Run update screenshots CI workflow label Nov 18, 2025
@OscarFava OscarFava removed the update-screenshots Run update screenshots CI workflow label Nov 19, 2025
@dwivedisachin
Copy link
Contributor

Tested LGTM!! 🚀

@dwivedisachin dwivedisachin merged commit 211b85a into Vonage:develop Nov 19, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants