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

Voice Broadcast - Listening #7387

Merged
merged 11 commits into from
Oct 18, 2022

Conversation

Florian14
Copy link
Contributor

@Florian14 Florian14 commented Oct 17, 2022

Type of change

  • WIP Feature
  • Bugfix
  • Technical
  • Other :

Content

Add the ability to listen to a voice broadcast.
For the moment, the incoming voice messages are not added to the queue when the listening is ongoing.

Motivation and context

Continue #7127

Screenshots / GIFs

Tests

  • Enable the voice broadcast feature flag
  • Start a voice broadcast in a room
  • Verify that you can listen to a voice broadcast from another account when it has ended
  • Verify that you can listen to an ongoing voice broadcast from another account (only for the already received events)
  • Verify that the UI is correctly updated

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@Florian14 Florian14 force-pushed the feature/fre/voice_broadcast_start_listening branch 2 times, most recently from 49f6427 to 441001b Compare October 18, 2022 06:59
@Florian14 Florian14 requested review from a team and jmartinesp and removed request for a team October 18, 2022 07:00
@Florian14 Florian14 marked this pull request as ready for review October 18, 2022 07:00
@Florian14 Florian14 added the Z-NextRelease For issues and PRs which should be included in the NextRelease. label Oct 18, 2022
@jmartinesp
Copy link
Member

jmartinesp commented Oct 18, 2022

I still haven't looked at the code, but one thing I've noticed is that the broadcast pauses as soon as the device gets locked, so maybe it would be nice to keep the screen on while you're recording?

Also, a question: how long does it take the voice broadcast to start playing? I tried listening to it ~5s after it had started and nothing came out of the speakers, I thought it was broken. Then I tried again like 20s later and it was working.

Besides that it seems to be working fine 👍 . I'll take a look at the code and approve if I don't find any major issues.

Copy link
Member

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

LGTM! Just the couple of questions/points mentioned above, but they're not blockers.

Keeping the screen on while recording or listening would be nice for UX if it can be implemented quickly.

@Florian14
Copy link
Contributor Author

@jmartinesp Thanks for your quick review! In this PR, the duration of the voice messages is 30 sec. There is no indicator in the UI about the availability of the messages, I agree it is not nice from a UX point of view.
I add the recording improvement to my todo list 👍🏻

@Florian14 Florian14 force-pushed the feature/fre/voice_broadcast_start_listening branch from 441001b to d53ad43 Compare October 18, 2022 14:23
Base automatically changed from feature/fre/voice_broadcast_start_record to develop October 18, 2022 14:43
@sonarcloud
Copy link

sonarcloud bot commented Oct 18, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Florian14 Florian14 merged commit 0dad78a into develop Oct 18, 2022
@Florian14 Florian14 deleted the feature/fre/voice_broadcast_start_listening branch October 18, 2022 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-NextRelease For issues and PRs which should be included in the NextRelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants