Support for Voice Call only (no video), parity with web#5995
Support for Voice Call only (no video), parity with web#5995BillCarsonFr merged 21 commits intodevelopfrom
Conversation
|
|
What is the status of this PR? |
|
@BillCarsonFr bump- I've tried this and it works fine. What's left to do before this can be merged? |
What is missing is mainly the incoming part of the functionality (so a ruma PR). |
|
@BillCarsonFr this feature is so underrated and indispensable to have this app look more familiar to those newcomers that aren't especially tech-saavy... Thank you very much for the effort. Will gladly pay for a few ko-fis |
|
Thank you, really looking forward to this one. |
|
@BillCarsonFr is there any issue opened on Ruma's repo as blocker for this one? |
|
Some progress on the ruma side ruma/ruma#2383 |
|
The rust sdk bindings are on their way matrix-org/matrix-rust-sdk#6207 |
|
Awesome @BillCarsonFr , great job! Thank you very much. |
a741863 to
5491040
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #5995 +/- ##
===========================================
+ Coverage 81.27% 81.28% +0.01%
===========================================
Files 2576 2577 +1
Lines 70105 70203 +98
Branches 9002 9016 +14
===========================================
+ Hits 56975 57062 +87
- Misses 9777 9786 +9
- Partials 3353 3355 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Ready for a first review! |
| ) | ||
| } | ||
|
|
||
| internal fun aIncomingCallScreenState( |
There was a problem hiding this comment.
Please rename the function to aCallNotificationData
| // Only show voice call in DMs | ||
| if (roomCallState.isDM) { | ||
| IconButton( | ||
| modifier = modifier, |
There was a problem hiding this comment.
You cannot use the modifier twice. I think you can just remove this line. The CI should detect this too.
There was a problem hiding this comment.
ok, I just used it in the row then. I see that in several places there are no modifiers on Buttons. 4406b50
There was a problem hiding this comment.
Thanks. Weird that the CI did not complain about this issue. I'll double check that.
| } | ||
| } | ||
| IconButton( | ||
| modifier = modifier, |
There was a problem hiding this comment.
Same remark, please remove this line.
| onSendLocationClick = callback::navigateToSendLocation, | ||
| onCreatePollClick = callback::navigateToCreatePoll, | ||
| onJoinCallClick = { callback.navigateToRoomCall(room.roomId) }, | ||
| onJoinCallClick = { voiceOnly -> |
There was a problem hiding this comment.
There is a mix between voiceOnly and isAudioCall, maybe we could use a single name across the codebase? I'd vote to only use isAudioCall
| canJoinCall = canJoinCall, | ||
| isUserInTheCall = isUserInTheCall, | ||
| isUserLocallyInTheCall = isUserLocallyInTheCall, | ||
| // TODO resolve intent while the call is ongoing |
There was a problem hiding this comment.
This TODO will not be handled in this PR?
Regarding join an existing call voice vs Video, there is a separate issue #6291 This PR is just based on the notification event, e.g ringing UX. For the other issue - web cannot answer - have you created an issue on web with a rageshake? |
Ok, thanks!
I just checked and it's happening for video calls too, maybe that test user doesn't have permission to send the notify call event in those rooms 👀 . |
|
I have also checked the permission management, and with a fresh install (i.e. with no specific granted permissions), the application only request for the microphone permission when starting a voice call. If the video is toggled on later, the camera permission is requested. So it's working as expected, that's nice! |
|




Changes
Depends on rust SDK matrix-org/matrix-rust-sdk#6003
m.call.intentin notification event MSC4075Content
Use the new intent system to signal the intention to make an audio only call.
Motivation and context
Screenshots / GIFs
New notification UI
Tests
Tested devices
Checklist