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

Increase max. length of voice message recordings to 5m #7582

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

Johennes
Copy link
Contributor

@Johennes Johennes commented Jun 7, 2023

I tested this on an iPhone X and the experience felt acceptable. When recording a 5m voice message, it took two or three seconds for it to display in the timeline after pressing the send button. This isn't particularly great UX but struck me as acceptable given that the current limit appears to be quite limiting for parts of our user base (see the discussion in #5415). @jakewb-b curious if you'd be ok with this.

@sonarcloud
Copy link

sonarcloud bot commented Jun 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

@jakewb-b jakewb-b left a comment

Choose a reason for hiding this comment

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

I'd be comfortable with that. It's a trade-off between two 'not great' experiences, but overall I think the impact of the current severe time restriction is greater than the impact of a few seconds delay when sending.

@Johennes Johennes marked this pull request as ready for review June 7, 2023 08:13
@Johennes Johennes requested review from a team and nimau and removed request for a team June 7, 2023 08:13
@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.02 🎉

Comparison is base (1e5809d) 12.34% compared to head (1167086) 12.36%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7582      +/-   ##
===========================================
+ Coverage    12.34%   12.36%   +0.02%     
===========================================
  Files         1648     1648              
  Lines       163634   163637       +3     
  Branches     67188    67194       +6     
===========================================
+ Hits         20204    20238      +34     
+ Misses      142765   142734      -31     
  Partials       665      665              
Flag Coverage Δ
uitests 55.06% <ø> (+0.01%) ⬆️
unittests 6.21% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...es/Room/VoiceMessages/VoiceMessageController.swift 0.00% <ø> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jacotec
Copy link

jacotec commented Jun 7, 2023

Highly appreciated from my side! 5 minutes may calm down users a bit while waiting for Element-X ;-)

Copy link
Contributor

@nimau nimau left a comment

Choose a reason for hiding this comment

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

SGTM

@Johennes Johennes merged commit b10b75c into develop Jun 7, 2023
@Johennes Johennes deleted the johannes/vm-length branch June 7, 2023 12:03
@Johennes
Copy link
Contributor Author

Johennes commented Jun 7, 2023

@jacotec the next release is currently scheduled for the week of June 19th.

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