Skip to content

Capabilities change handler#3485

Merged
mgamis-msft merged 10 commits intomainfrom
mgamis/capabilities-change-handler
Aug 24, 2023
Merged

Capabilities change handler#3485
mgamis-msft merged 10 commits intomainfrom
mgamis/capabilities-change-handler

Conversation

@mgamis-msft
Copy link
Copy Markdown
Contributor

@mgamis-msft mgamis-msft commented Aug 21, 2023

What

  • Added capabilitiesChanged to CallAdapter.
  • Turning off camera when capability for camera is lost

Why

  • Allow Contoso to handle the capabilitiesChanged event.
  • Copying behaviour of Teams to turn off camera when capability for camera is lost

How Tested

Local Interop testing.
https://github.com/Azure/communication-ui-library/assets/79475487/7b1b93b6-1399-4ede-a8ac-c4516dc08c53

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@mgamis-msft mgamis-msft requested review from a team as code owners August 21, 2023 06:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 21, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-zgpgzyesbs.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 21, 2023 •
edited
Loading

CallWithChat bundle size is increased❗.

  • Current size: 7066845
  • Base size: 7066365
  • Diff size: 480

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 21, 2023 •
edited
Loading

Chat bundle size is not changed.

  • Current size: 2398957
  • Base size: 2398957
  • Diff size: 0

alkwa-msft
alkwa-msft reviewed Aug 21, 2023
View reviewed changes
Comment thread packages/react-composites/src/composites/CallComposite/adapter/AzureCommunicationCallAdapter.ts Outdated
Copy link
Copy Markdown
Contributor

@alkwa-msft alkwa-msft Aug 21, 2023

Choose a reason for hiding this comment

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

is newValue ever undefined?

Copy link
Copy Markdown
Contributor Author

@mgamis-msft mgamis-msft Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

newValue is a Record and can't be undefined but can be empty. But I should be checking that turnVideoOn is in the Record with an isPresent value of false explicitly . I should not be potentially coercing a undefined to false. Fixed.

alkwa-msft
alkwa-msft reviewed Aug 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@alkwa-msft alkwa-msft left a comment

Choose a reason for hiding this comment

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

can we get a guarantee that when we are chaining calls that could be undefined with ? that we won't break at runtime? I would prefer to not have a repeat when we tried to deploy self host with raised hand and it broke due to us not checking correctly for capabilties?

Base automatically changed from upgrade-calling-beta-to-1.16.2-beta.1 to main August 21, 2023 21:18
@mgamis-msft mgamis-msft force-pushed the mgamis/capabilities-change-handler branch from 8609471 to bd60dab Compare August 22, 2023 18:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 22, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-rtrswxmcxi.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 22, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-vhrqcvkikh.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 22, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-bprgpxmmsv.chromatic.com/

dmceachernmsft
dmceachernmsft approved these changes Aug 22, 2023
View reviewed changes
Comment thread packages/react-composites/src/composites/CallComposite/adapter/AzureCommunicationCallAdapter.ts
/* @conditional-compile-remove(capabilities) */
private capabilitiesChanged(data: CapabilitiesChangeInfo): void {
if (data.newValue.turnVideoOn?.isPresent === false) {
this.stopCamera();
Copy link
Copy Markdown
Member

@dmceachernmsft dmceachernmsft Aug 22, 2023

Choose a reason for hiding this comment

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

Should we also be looking at the microphone state here to? Mute the participant if the capability is removed?

Copy link
Copy Markdown
Contributor Author

@mgamis-msft mgamis-msft Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The calling SDK mutes the mic when the unmute capability changed to not present but yeah we should also make sure on our side. Updated.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 23, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-wugvgqicvg.chromatic.com/

prabhjot-msft
prabhjot-msft approved these changes Aug 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@prabhjot-msft prabhjot-msft left a comment

Choose a reason for hiding this comment

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

looks good, hoping the existing comments are resolved before merge!

@mgamis-msft mgamis-msft enabled auto-merge (squash) August 24, 2023 01:38
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 24, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-sdkzxphrzx.chromatic.com/

@mgamis-msft mgamis-msft disabled auto-merge August 24, 2023 01:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 24, 2023

Storybook URL https://60c7ae6891f0e90039d7cd54-qwwzzhexjr.chromatic.com/

@mgamis-msft mgamis-msft merged commit 1911608 into main Aug 24, 2023
@mgamis-msft mgamis-msft deleted the mgamis/capabilities-change-handler branch August 24, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmceachernmsft dmceachernmsft dmceachernmsft approved these changes

@prabhjot-msft prabhjot-msft prabhjot-msft approved these changes

@JamesBurnside JamesBurnside Awaiting requested review from JamesBurnside JamesBurnside is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@carocao-msft carocao-msft Awaiting requested review from carocao-msft carocao-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@edwardlee-msft edwardlee-msft Awaiting requested review from edwardlee-msft edwardlee-msft is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-1

@PorterNan PorterNan Awaiting requested review from PorterNan PorterNan is a code owner automatically assigned from Azure/acs-ui-web-calling-reviewer-2

@AmyL219 AmyL219 Awaiting requested review from AmyL219 AmyL219 is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@jimchou-dev jimchou-dev Awaiting requested review from jimchou-dev jimchou-dev is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@vhuseinova-msft vhuseinova-msft Awaiting requested review from vhuseinova-msft vhuseinova-msft is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@Leah-Xia-Microsoft Leah-Xia-Microsoft Awaiting requested review from Leah-Xia-Microsoft Leah-Xia-Microsoft is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@jpeng-ms jpeng-ms Awaiting requested review from jpeng-ms jpeng-ms is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-1

@waynemo waynemo Awaiting requested review from waynemo waynemo is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

@emlynmac emlynmac Awaiting requested review from emlynmac emlynmac is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

@JoshuaLai JoshuaLai Awaiting requested review from JoshuaLai JoshuaLai is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

@palatter palatter Awaiting requested review from palatter palatter is a code owner automatically assigned from Azure/acs-ui-web-chat-reviewer-2

+1 more reviewer

@alkwa-msft alkwa-msft alkwa-msft approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dmceachernmsft dmceachernmsft

@prabhjot-msft prabhjot-msft

Labels

breaking-change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mgamis-msft @alkwa-msft @dmceachernmsft @prabhjot-msft @JamesBurnside
⚡