-
Notifications
You must be signed in to change notification settings - Fork 742
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
Fixes voice call button disappearing in DM rooms with more than 2 members #5548
Conversation
Unit Test Results106 files + 8 106 suites +8 1m 6s ⏱️ -1s Results for commit bfd31de. ± Comparison against base commit e0b93c2. This pull request removes 12 and adds 20 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
@@ -87,7 +87,7 @@ data class RoomDetailViewState( | |||
rootThreadEventId = args.threadTimelineArgs?.rootThreadEventId | |||
) | |||
|
|||
fun isWebRTCCallOptionAvailable() = (asyncRoomSummary.invoke()?.joinedMembersCount ?: 0) <= 2 | |||
fun isWebRTCCallOptionAvailable() = asyncRoomSummary.invoke()?.isDirect ?: true |
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 don't have enough context about our support for more than 2 people. Web client is creating a Jitsi video call widget even if you click to voice call button. @ganfra what do you think?
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.
Yes WebRtc is only working with one opponent, otherwise it should be a jitsi call (with video disabled then)
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 suppose this must be why it was disabled previously. Thanks for the context, I'll reapproach this!
Edit: Upon looking at the code, the call should still be working as a Jitsi audio call out-the-box. I renamed the above function to be more agnostic towards the type of call
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.
LGTM! Thanks for the update.
Type of change
Content
Changes the condition for the voice call option on the action bar showing from being in a room of 2 or less people to simply being in a direct message room.
Motivation and context
Closes #4762
Quoting conversation in the issue (re current behaviour of voice call not showing):
"this is not a normal behaviour for a DM based group chat (using native VoIP system). We should have both voice and video calls displayed.
For "group rooms" e.g: the large Matrix community room, we currently use Jitsi and so only conference calls should be displayed with a video button. The conference calls are controlled by admins with permissions."
Screenshots / GIFs
Tests
Tested devices
Checklist