-
Notifications
You must be signed in to change notification settings - Fork 73
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
Add composite option to allow join the call without microphone #5671
base: main
Are you sure you want to change the base?
Conversation
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-gcdaywmxge.chromatic.com/ |
@azure/communication-react jest test coverage for stable.
|
@azure/communication-react jest test coverage for beta.
|
@@ -797,6 +797,7 @@ export type CallCompositeOptions = { | |||
spotlight?: { | |||
hideSpotlightButtons?: boolean; | |||
}; | |||
skipMicCheck?: boolean; |
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.
This name needs to be more specific, it doesn't say what microphone checks are skipped, and could conflict with other microphone checks in the Composite experience. We should also try to always use microphone
instead of mic
. Consider:
skipMicCheck?: boolean; | |
joinCallOptions: { | |
doNotBlockOnMicrophoneAccess: boolean; | |
} |
^That suggestion is a tad verbose
skipMicCheck?: boolean; | |
joinCallOptions: { | |
skipMicrophoneCheck: boolean; | |
} |
Would work also, but consider if we had additional options in future such as the ability to block on microphone permission only but allow no microphone plugged in, or the ability to block on not just mic but camera as well we need a naming scheme that is consistent for future scenarios, perhaps also:
skipMicCheck?: boolean; | |
joinCallOptions: { | |
microphoneCheck: 'blockOnAccess' /* default */ | 'blockOnEnumeration' /*future*/ | 'skip', | |
cameraCheck: 'blockOnAccess' | 'blockOnFaceDetection' | // just some examples | |
} |
Not sure what the beahvior is today, is it just on permission or does there have to be an available microphone as well?
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.
Of these options I think I like the last one best as it gives more control to Contoso
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.
PR Overview
This PR introduces a new composite option (skipMicCheck) to allow users to join a call without requiring a microphone. Key changes include:
- Adding the skipMicCheck property to the composite options and associated props.
- Updating logic to conditionally disable the start call button based on skipMicCheck.
- Propagating the new skipMicCheck option through CallComposite, CallWithChatComposite, and API documentation.
Reviewed Changes
File | Description |
---|---|
packages/react-composites/src/composites/CallComposite/pages/ConfigurationPage.tsx | Added skipMicCheck prop and updated logic for enabling/disabling the start call button. |
packages/react-composites/src/composites/CallWithChatComposite/CallWithChatComposite.tsx | Extended composite options to include skipMicCheck and passed it to the CallWithChatScreen. |
packages/react-composites/src/composites/CallComposite/CallComposite.tsx | Updated CallComposite options to include skipMicCheck and forwarded it to the configuration page. |
packages/communication-react/review/stable/communication-react.api.md | Updated API documentation to include skipMicCheck in composite option types (stable). |
packages/communication-react/review/beta/communication-react.api.md | Updated API documentation to include skipMicCheck in composite option types (beta). |
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
packages/react-composites/src/composites/CallComposite/pages/ConfigurationPage.tsx:149
- Consider adding tests to verify that when skipMicCheck is true, the start call button is enabled regardless of the microphone permission state or available microphones.
let disableStartCallButton = (!microphonePermissionGranted || microphones?.length === 0) && !skipMicCheck;
packages/react-composites/src/composites/CallWithChatComposite/CallWithChatComposite.tsx:548
- Ensure that tests cover the scenario where skipMicCheck is provided so that its propagation through composite options is validated.
spotlight: props.spotlight,
packages/react-composites/src/composites/CallComposite/CallComposite.tsx:564
- Add tests to cover the new skipMicCheck option in composite configurations to ensure that it correctly influences call behavior.
skipMicCheck={props.options?.skipMicCheck}
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-fhrjrablmr.chromatic.com/ |
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-nyppxeqvqu.chromatic.com/ |
CallWithChat bundle size is not changed.
|
Calling bundle size is not changed.
|
Chat bundle size is not changed.
|
Storybook 8 URL https://60c7ae6891f0e90039d7cd54-wlbhlwkcdp.chromatic.com/ |
What
Add composite option to allow join the call without microphone
Why
How Tested
Process & policy checklist
Is this a breaking change?