VIDSOL-211: handle permissions if user disables enables camera mic #283
Conversation
johnny-quesada-developer
changed the base branch from
main
to
behei-vonage/vidsol-5-sub-warnings
There was a problem hiding this comment.
Pull request overview
This pull request implements proper handling of camera and microphone permissions based on application configuration, and adds skeleton screens to improve UX during app initialization. The changes ensure that camera/microphone streams are only initialized when both the configuration allows it AND the feature is enabled by the user.
Key Changes:
- Implements conditional media stream initialization based on app configuration
- Adds skeleton screens for loading states in WaitingRoom and MeetingRoom
- Refactors configuration access patterns throughout the app
- Introduces new storage keys for tracking audio/video source enabled state
- Creates reusable UI components (Box, Stack, TextField, Typography, etc.) as MUI wrappers
Reviewed changes
Copilot reviewed 292 out of 447 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/pages/WaitingRoom/WaitingRoom.tsx |
Adds app config checks before initializing camera/mic streams |
frontend/src/pages/MeetingRoom/MeetingRoom.tsx |
Implements config-based background publisher initialization |
frontend/src/components/WaitingRoom/VideoContainer/VideoContainer.tsx |
Conditionally renders camera/mic controls based on app config |
frontend/src/utils/storage.ts |
Adds storage keys for tracking enabled state of audio/video sources |
frontend/ui/ |
Creates new reusable UI component wrappers for MUI components |
frontend/src/test/providers/ |
Adds test provider utilities for better test isolation |
integration-tests/ |
Migrates to nx project structure and updates test selectors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -102,37 +127,40 @@ const WaitingRoom = (): ReactElement => { | |||
| setOpenVideoInput(false); | |||
| }; | |||
|
|
|||
| // [TODO]: check-styles 'flex size-full flex-col bg-white' | |||
There was a problem hiding this comment.
Remove TODO comment or complete the style check. This appears to be a leftover note that should either be addressed or removed before merging.
| hasAudio={isAudioEnabled} | ||
| indicatorStyle={audioIndicatorStyle} | ||
| indicatorColor="white" | ||
| stream={publisher?.stream} |
There was a problem hiding this comment.
New prop stream added to AudioIndicator component lacks test coverage. Add tests to verify this prop is correctly passed and used.
| // eslint-disable-next-line @typescript-eslint/await-thenable | ||
| await act(() => { | ||
| rerender(<WaitingRoomWithProviders />); |
There was a problem hiding this comment.
Investigation needed: the test requires awaiting a non-thenable. This suggests either a timing issue in the test or incorrect async handling. Should be investigated and properly fixed rather than suppressed.
| // eslint-disable-next-line @typescript-eslint/await-thenable | |
| await act(() => { | |
| rerender(<WaitingRoomWithProviders />); | |
| await act(async () => { | |
| rerender(<WaitingRoomWithProviders />); | |
| await Promise.resolve(); |
| @@ -168,19 +164,62 @@ describe('WaitingRoom', () => { | |||
| const input = screen.getByPlaceholderText('Enter your name'); | |||
| await user.type(input, 'Betsey Trotwood'); | |||
| expect(input).toHaveValue('Betsey Trotwood'); | |||
|
|
|||
| // TODO: pending check that the enter was called | |||
There was a problem hiding this comment.
Incomplete test: verification that Enter key triggers navigation is missing. Complete this test assertion.
| /** | ||
| * TODO: We still need to create provider wrappers for the following contexts: | ||
| * | ||
| * AudioOutputProvider, | ||
| * BackgroundPublisherProvider, | ||
| * PreviewPublisherProvider, | ||
| * PublisherProvider, | ||
| * RoomProvider | ||
| * | ||
| * Right now we are mocking all those context which downgrades the quality of our tests. | ||
| */ |
There was a problem hiding this comment.
Missing test provider wrappers: Create provider wrappers for AudioOutputProvider, BackgroundPublisherProvider, PreviewPublisherProvider, PublisherProvider, and RoomProvider to improve test quality and reduce reliance on mocks.
johnny-quesada-developer
force-pushed
the
johnny/VIDSOL-211-Handle-permissions-if-user-disablesEnables-cameraMic
branch
from
b553386 to
a9cc688
Compare
johnny-quesada-developer
changed the base branch from
behei-vonage/vidsol-5-sub-warnings
to
develop
johnny-quesada-developer
force-pushed
the
johnny/VIDSOL-211-Handle-permissions-if-user-disablesEnables-cameraMic
branch
17 times, most recently
from
a4f0957 to
11cb3e3
Compare
- removing noise - adding scripts to debug locally johnny/VIDSOL-211-advances johnny/VIDSOL-211-revert camera/mic buttons changes revert to later cause it breaks a lot of integration tests johnny/VIDSOL-211-linter configurations and cleanups johnny/VIDSOL-211-fixing infinite loop of retries on publish johnny/VIDSOL-211-refinements and unit test johnny/VIDSOL-211-removing opera johnny/VIDSOL-211-Handle-permissions-if-user-disablesEnables-cameraMic
johnny-quesada-developer
force-pushed
the
johnny/VIDSOL-211-Handle-permissions-if-user-disablesEnables-cameraMic
branch
from
11cb3e3 to
a896732
Compare
|
What is this PR doing?
Fixes several issues related to how the configurable features work and how the camera and microphone streams are initialized.
If the app configuration says the camera is not allowed, we should not enable it. Similarly, if the camera is turned off, the application should stop requesting access to it.
We also added skeletons to display while the app configuration is being fetched to avoid blinking UI/UX. The configuration will eventually be embedded in the app bundle, but skeletons will still be useful for other potential latencies before the connection is established.
How should this be manually tested?
What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDSOL-211
Checklist
[x] Branch is based on
develop(notmain).[x] Resolves a
Known Issue.[ ] If yes, did you remove the item from the
docs/KNOWN_ISSUES.md?[ ] Resolves an item reported in
Issues.If yes, which issue? Issue Number?