Skip to content

fix: joinCall set correct mute state based on microphoneOn parameter#2131

Merged
JamesBurnside merged 9 commits intoAzure:mainfrom
bfwg:joinCall-microphoneOn-fix
Aug 2, 2022
Merged

fix: joinCall set correct mute state based on microphoneOn parameter#2131
JamesBurnside merged 9 commits intoAzure:mainfrom
bfwg:joinCall-microphoneOn-fix

Conversation

@bfwg
Copy link
Copy Markdown
Contributor

@bfwg bfwg commented Jul 26, 2022

What

Fixed a bug where the initial muted state is not being set correctly.

Why

When microphoneOn is true, muted will be true which is backward of what the code is intended to do.

How Tested

I tested calling joinCall with microphoneOn parameter true and the call will start muted which is incorrect.

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.

Copy link
Copy Markdown
Contributor

@prprabhu-ms prprabhu-ms left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

I checked in the rest of the codebase and turns out we don't ever call joinCall with microphoneOn set (to either true or false). That is why things were not on fire already.

Thanks for catching this.


return this.teeErrorToEventEmitter(() => {
const audioOptions: AudioOptions = { muted: microphoneOn ?? !this.getState().isLocalPreviewMicrophoneEnabled };
const audioOptions: AudioOptions = { muted: !microphoneOn ?? !this.getState().isLocalPreviewMicrophoneEnabled };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

microphoneOn is optional. If we check for falsiness, calling joinCall() without any arguments will cause muted to be set to true.

So, fallback on undefined, and then check for falsiness...

Suggested change
const audioOptions: AudioOptions = { muted: !microphoneOn ?? !this.getState().isLocalPreviewMicrophoneEnabled };
const audioOptions: AudioOptions = { muted: !(microphoneOn ?? this.getState().isLocalPreviewMicrophoneEnabled) };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion. I've fixed my code.

@prprabhu-ms
Copy link
Copy Markdown
Contributor

Do you mind running rush changelog and providing a changelog for this PR?

The type should be "patch" (as this is a bugfix). The PR title is a good change description.

@prprabhu-ms
Copy link
Copy Markdown
Contributor

I'm trying to figure out why chromatic is unhappy.

@ghost
Copy link
Copy Markdown

ghost commented Jul 27, 2022

CLA assistant check
All CLA requirements met.

@bfwg
Copy link
Copy Markdown
Contributor Author

bfwg commented Jul 27, 2022

Done.

@JamesBurnside JamesBurnside merged commit 6f13869 into Azure:main Aug 2, 2022
@bfwg bfwg deleted the joinCall-microphoneOn-fix branch August 2, 2022 18:50
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.

4 participants