Skip to content
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

Merged
merged 6 commits into from
Mar 29, 2022

Conversation

ericdecanini
Copy link
Contributor

@ericdecanini ericdecanini commented Mar 15, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

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

1:1 DM Group DM Non-direct chatroom
image image image

Tests

  • Go to a DM room with 1 other member. See VoIP button visible
  • Go to a DM room with at least 2 other members. See VoIP button visible
  • Go to a non-direct room. See VoIP button hidden

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 10

Checklist

@github-actions
Copy link

github-actions bot commented Mar 15, 2022

Unit Test Results

106 files  +  8  106 suites  +8   1m 6s ⏱️ -1s
188 tests +  8  188 ✔️ +  8  0 💤 ±0  0 ±0 
622 runs  +32  622 ✔️ +32  0 💤 ±0  0 ±0 

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.
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given a selected picture when handling save selected profile picture then updates upstream avatar and completes personalization
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given changing profile picture is not supported when updating display name then updates upstream user display name and completes personalization
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given changing profile picture is supported when updating display name then updates upstream user display name and moves to choose profile picture
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given homeserver does not support personalisation when registering account then updates state and emits account created event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given no selected picture when saving selected profile picture then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given only supports changing profile picture when handling PersonalizeProfile then emits contents choose profile picture
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given supports changing display name when handling PersonalizeProfile then emits contents choose display name
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given upstream failure when handling display name update then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given upstream update avatar fails when saving selected profile picture then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ when handling PostViewEvent then emits contents as view event
…
im.vector.app.features.location.domain.usecase.CompareLocationsUseCaseTest ‑ given 2 far away locations when calling execute then these locations are considered as not equal
im.vector.app.features.location.domain.usecase.CompareLocationsUseCaseTest ‑ given 2 very near locations when calling execute then these locations are considered as equal
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given a selected picture, when handling save selected profile picture, then updates upstream avatar and completes personalization
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given changing profile picture is not supported, when updating display name, then updates upstream user display name and completes personalization
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given changing profile picture is supported, when updating display name, then updates upstream user display name and moves to choose profile picture
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given no selected picture, when saving selected profile picture, then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given only supports changing profile picture, when handling PersonalizeProfile, then emits contents choose profile picture
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given register action ignores result, when handling action, then does nothing on success
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given register action is non loadable, when handling action, then posts next steps without loading
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given register action requires more steps, when handling action, then posts next steps
…

♻️ This comment has been updated with latest results.

@ericdecanini ericdecanini marked this pull request as ready for review March 15, 2022 19:49
@ericdecanini ericdecanini requested review from a team and onurays and removed request for a team March 15, 2022 19:49
@@ -87,7 +87,7 @@ data class RoomDetailViewState(
rootThreadEventId = args.threadTimelineArgs?.rootThreadEventId
)

fun isWebRTCCallOptionAvailable() = (asyncRoomSummary.invoke()?.joinedMembersCount ?: 0) <= 2
fun isWebRTCCallOptionAvailable() = asyncRoomSummary.invoke()?.isDirect ?: true
Copy link
Contributor

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?

Copy link
Member

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)

Copy link
Contributor Author

@ericdecanini ericdecanini Mar 22, 2022

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

@ericdecanini ericdecanini requested a review from onurays March 22, 2022 16:58
Copy link
Contributor

@onurays onurays left a 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.

@ericdecanini ericdecanini merged commit 9c333c9 into develop Mar 29, 2022
@ericdecanini ericdecanini deleted the bugfix/eric/call-button-disappearing branch March 29, 2022 14:10
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.

Voice call button disappears when there's more than 2 people in the room.
3 participants