-
Notifications
You must be signed in to change notification settings - Fork 10
VIDSOL-143: Implement configuration file, VIDSOL-195: Resolve camera LED bug turning on when not publishing video #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
013ea86
first things first
cpettet 44c7ab0
working for some stuff
cpettet 4260439
some renaming
cpettet 711f4b6
enable/disable camera
cpettet 2d027d9
video resolution
cpettet 85fcbdc
less redundant
cpettet 69680ca
..
cpettet e695b8d
test fixes
cpettet 26ebf42
should work on gha
cpettet dc29859
that type
cpettet 335a47f
revert and redo
cpettet 46f745f
lint issues
cpettet 4e30edd
should be good?
cpettet a245d74
should work on gha and local
cpettet de1d224
ans configurable
cpettet dda6c9e
test
cpettet cf5f406
audio/video control button tests
cpettet 9b90d9d
fix lint
cpettet 020364f
fix tests
cpettet 7f4d686
tests for config
cpettet 9bef624
enable-disable waiting room
cpettet 6e69991
cleaner implementation
cpettet 3142edc
audio and video on join
cpettet 3269dca
background publisher taken care of
cpettet 0889143
configured control panel
cpettet f4c15ab
fix that test
cpettet 587471c
too excited
cpettet 6170788
participant-list
cpettet 3e10a68
lint
cpettet 01713d7
config file
cpettet 84e7ec3
resolve another bug
cpettet f5fa301
fixed tests
cpettet 4d478d5
chat button
cpettet f37c7a8
screen-sharing
cpettet 8380877
archiving button. layoutMode --> defaultLayoutMode
cpettet 1c2660d
captions button
cpettet 7f6e5c0
showemoji
cpettet 70f8369
rename some flags
cpettet 2801140
renaming
cpettet f4f5999
fixing tests
cpettet a423cea
merge changes
cpettet ec9c5f0
fix tests. use en.json
cpettet 2c0dd1d
renaming
cpettet d5e4db6
fix
cpettet c428a25
test fix
cpettet add0885
show participant list
cpettet 5859fa7
new test file. renaming some tests for consistency
cpettet 3bddf3c
a review from yours truly
cpettet ebed32a
my manual testing
cpettet e725537
not enough ../
cpettet 39722fa
ghc suggests
cpettet 96d8dbf
for the meeting room
cpettet 40c6585
some nits
cpettet 170018e
single not double
cpettet 20d56e0
helper instead
cpettet e71ad04
the great renaming
cpettet 0a4bb74
better config fetch handling
cpettet 5ab05b2
missing jsdoc
cpettet c0d5791
simpler solution to solve the issue
cpettet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "videoSettings": { | ||
| "allowBackgroundEffects": true, | ||
| "allowCameraControl": true, | ||
| "allowVideoOnJoin": true, | ||
| "defaultResolution": "1280x720" | ||
| }, | ||
| "audioSettings": { | ||
| "allowAdvancedNoiseSuppression": true, | ||
| "allowAudioOnJoin": true, | ||
| "allowMicrophoneControl": true | ||
| }, | ||
| "waitingRoomSettings": { | ||
| "allowDeviceSelection": true | ||
| }, | ||
| "meetingRoomSettings": { | ||
| "allowArchiving": true, | ||
| "allowCaptions": true, | ||
| "allowChat": true, | ||
| "allowDeviceSelection": true, | ||
| "allowEmojis": true, | ||
| "allowScreenShare": true, | ||
| "defaultLayoutMode": "active-speaker", | ||
| "showParticipantList": true | ||
| } | ||
| } | ||
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { createContext, ReactNode, useMemo } from 'react'; | ||
| import useConfig, { defaultConfig } from './useConfig'; | ||
|
|
||
| export type ConfigProviderProps = { | ||
| children: ReactNode; | ||
| }; | ||
|
|
||
| export type ConfigContextType = ReturnType<typeof useConfig>; | ||
|
|
||
| export const ConfigContext = createContext<ConfigContextType>({ | ||
| audioSettings: defaultConfig.audioSettings, | ||
| meetingRoomSettings: defaultConfig.meetingRoomSettings, | ||
| waitingRoomSettings: defaultConfig.waitingRoomSettings, | ||
| videoSettings: defaultConfig.videoSettings, | ||
| }); | ||
|
|
||
| /** | ||
| * ConfigProvider - React Context Provider for ConfigContext | ||
| * ConfigContext contains all application configuration including video settings, audio settings, | ||
| * waiting room settings, and meeting room settings loaded from config.json. | ||
| * We use Context to make the configuration available in many components across the app without | ||
| * prop drilling: https://react.dev/learn/passing-data-deeply-with-context#use-cases-for-context | ||
| * See useConfig.tsx for configuration structure and loading logic | ||
| * @param {ConfigProviderProps} props - The provider properties | ||
| * @property {ReactNode} children - The content to be rendered | ||
| * @returns {ConfigContext} a context provider for application configuration | ||
| */ | ||
| export const ConfigProvider = ({ children }: ConfigProviderProps) => { | ||
| const config = useConfig(); | ||
| const value = useMemo(() => config, [config]); | ||
|
|
||
| return <ConfigContext.Provider value={value}>{children}</ConfigContext.Provider>; | ||
| }; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| import useConfig, { defaultConfig } from './useConfig'; | ||
|
|
||
| export { defaultConfig }; | ||
| export default useConfig; |
131 changes: 131 additions & 0 deletions
131
frontend/src/Context/ConfigProvider/useConfig/useConfig.spec.tsx
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { renderHook, waitFor } from '@testing-library/react'; | ||
| import useConfig, { AppConfig } from './useConfig'; | ||
|
|
||
| describe('useConfig', () => { | ||
| let nativeFetch: typeof global.fetch; | ||
| const defaultConfig: AppConfig = { | ||
| videoSettings: { | ||
| allowCameraControl: true, | ||
| defaultResolution: '1280x720', | ||
| allowVideoOnJoin: true, | ||
| allowBackgroundEffects: true, | ||
| }, | ||
| audioSettings: { | ||
| allowAdvancedNoiseSuppression: true, | ||
| allowAudioOnJoin: true, | ||
| allowMicrophoneControl: true, | ||
| }, | ||
| waitingRoomSettings: { | ||
| allowDeviceSelection: true, | ||
| }, | ||
| meetingRoomSettings: { | ||
| allowArchiving: true, | ||
| allowCaptions: true, | ||
| allowChat: true, | ||
| allowDeviceSelection: true, | ||
| allowEmojis: true, | ||
| allowScreenShare: true, | ||
| defaultLayoutMode: 'active-speaker', | ||
| showParticipantList: true, | ||
| }, | ||
| }; | ||
| const consoleErrorSpy = vi.spyOn(console, 'error'); | ||
| const consoleInfoSpy = vi.spyOn(console, 'info'); | ||
|
|
||
| beforeAll(() => { | ||
| nativeFetch = global.fetch; | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| json: async () => ({}), | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.resetAllMocks(); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| global.fetch = nativeFetch; | ||
| }); | ||
|
|
||
| it('returns the default config when no config.json is loaded', async () => { | ||
| const { result } = renderHook(() => useConfig()); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current).toEqual(defaultConfig); | ||
| }); | ||
| }); | ||
|
|
||
| it('merges config.json values if loaded (mocked fetch)', async () => { | ||
| // All values in this config should override the defaultConfig | ||
| const mockConfig: AppConfig = { | ||
| videoSettings: { | ||
| allowCameraControl: false, | ||
| defaultResolution: '640x480', | ||
| allowVideoOnJoin: false, | ||
| allowBackgroundEffects: false, | ||
| }, | ||
| audioSettings: { | ||
| allowAdvancedNoiseSuppression: false, | ||
| allowAudioOnJoin: false, | ||
| allowMicrophoneControl: false, | ||
| }, | ||
| waitingRoomSettings: { | ||
| allowDeviceSelection: false, | ||
| }, | ||
| meetingRoomSettings: { | ||
| allowArchiving: false, | ||
| allowCaptions: false, | ||
| allowChat: false, | ||
| allowDeviceSelection: false, | ||
| allowEmojis: false, | ||
| allowScreenShare: false, | ||
| defaultLayoutMode: 'grid', | ||
| showParticipantList: false, | ||
| }, | ||
| }; | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| json: async () => mockConfig, | ||
| headers: { | ||
| get: () => 'application/json', | ||
| }, | ||
| }); | ||
| const { result } = renderHook(() => useConfig()); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current).toMatchObject(mockConfig); | ||
| }); | ||
| }); | ||
|
|
||
| it('falls back to defaultConfig if fetch fails', async () => { | ||
| const mockFetchError = new Error('mocking a failure to fetch'); | ||
| global.fetch = vi.fn().mockRejectedValue(mockFetchError); | ||
| const { result } = renderHook(() => useConfig()); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current).toEqual(defaultConfig); | ||
| }); | ||
| expect(consoleErrorSpy).toHaveBeenCalledWith('Error loading config:', expect.any(Error)); | ||
| }); | ||
|
|
||
| it('falls back to defaultConfig if no config.json is found', async () => { | ||
| global.fetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 404, | ||
| statusText: 'Not Found', | ||
| headers: { | ||
| get: () => 'text/html', | ||
| }, | ||
| }); | ||
| const { result } = renderHook(() => useConfig()); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current).toEqual(defaultConfig); | ||
| }); | ||
| expect(consoleInfoSpy).toHaveBeenCalledWith('No valid JSON found, using default config'); | ||
| expect(consoleErrorSpy).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest renaming it to "allowVideoEffects" because, while currently these effects apply only to the background, the same panel could potentially be used in the future for other effects (e.g., lighting correction, eye correction).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this as a team during design review and considered
Visual Effectsinstead ofBackground Effectsbefore deciding on this naming scheme. All of our components referenceBackgroundEffectsand everything is currently a background effect. Let's keep as-is for now, seems like premature optimization to rename this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok @cpettet !