Skip to content
This repository has been archived by the owner on Jun 7, 2020. It is now read-only.

[NEW] Support sending audio records #1061

Merged
merged 41 commits into from
Dec 13, 2017

Conversation

luismachado
Copy link
Contributor

@RocketChat/ios

I forked #762 (as no update was given since a month ago) so credit also to @falcaoaugustos (maybe it's best to update his PR with this one?)

Changes since the fork:

  • Visual feedback is different now (whole screen is blurred while recording and a timer is also present)
  • Feedback is now quicker as I replaced the button action for a tapgesturerecognizer (delayTouchesBegan was also removed)
  • Code is also up to date with the most recent commits.

falcaoaugustos and others added 30 commits October 9, 2017 13:56
- Visual feedback when recording audio
class AudioMessageRecorder: NSObject, AVAudioRecorderDelegate {

private var recorder: AVAudioRecorder?
private var recorderDelegate: AVAudioRecorderDelegate? = nil

Choose a reason for hiding this comment

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

Weak Delegate Violation: Delegates should be weak to avoid reference cycles. (weak_delegate)
Redundant Optional Initialization Violation: Initializing an optional variable with nil is redundant. (redundant_optional_initialization)

@rafaelks
Copy link
Contributor

@luismachado Looks like this one have conflicts? 🤔

@luismachado
Copy link
Contributor Author

@rafaelks Yeah also noticed that. Looking into it now.

@rafaelks rafaelks changed the title Feature/audio message [NEW] Support sending audio records Dec 11, 2017
@rafaelks rafaelks added this to the 1.8.0 milestone Dec 11, 2017
@codecov
Copy link

codecov bot commented Dec 11, 2017

Codecov Report

Merging #1061 into develop will decrease coverage by 0.52%.
The diff coverage is 10.62%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1061      +/-   ##
===========================================
- Coverage     44.6%   44.08%   -0.53%     
===========================================
  Files          252      254       +2     
  Lines        14078    14307     +229     
  Branches       785      785              
===========================================
+ Hits          6280     6307      +27     
- Misses        7335     7537     +202     
  Partials       463      463
Impacted Files Coverage Δ
Rocket.Chat/Views/Chat/RecordingAudioView.swift 0% <0%> (ø)
...Chat/Controllers/Chat/ChatControllerUploader.swift 0% <0%> (ø) ⬆️
...ket.Chat/Controllers/Chat/ChatViewController.swift 20.06% <19.51%> (+0.51%) ⬆️
Rocket.Chat/Helpers/AudioMessageRecorder.swift 4.1% <4.1%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a173228...aec9ba5. Read the comment docs.

… into feature/AudioMessage

# Conflicts:
#	Rocket.Chat.xcodeproj/project.pbxproj
#	Rocket.Chat/Controllers/Chat/ChatViewController.swift
@luismachado
Copy link
Contributor Author

@rafaelks should be without conflicts now. Let me know what you think :)

Copy link
Contributor

@rafaelks rafaelks left a comment

Choose a reason for hiding this comment

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

I did some testing on the app, is looking good! Will test a few more later and give you some feedbacks on the UI/usage. In the meantime, there's a few things to be changed on the code of the PR.

Thank you very much @luismachado!

@@ -1691,7 +1700,7 @@
TargetAttributes = {
41DF76DE1D2C50710028DBF8 = {
CreatedOnToolsVersion = 7.3.1;
DevelopmentTeam = S6UPZG7ZR3;
DevelopmentTeam = MJHBLUH3VK;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this change?

@@ -2543,12 +2555,12 @@
CODE_SIGN_ENTITLEMENTS = Rocket.Chat/Rocket.Chat.entitlements;
CODE_SIGN_IDENTITY = "iPhone Developer";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer";
DEVELOPMENT_TEAM = S6UPZG7ZR3;
DEVELOPMENT_TEAM = MJHBLUH3VK;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this change?

INFOPLIST_FILE = Rocket.Chat/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 10.0;
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks";
OTHER_SWIFT_FLAGS = "$(inherited) -D DEBUG \"-D\" \"COCOAPODS\"";
PRODUCT_BUNDLE_IDENTIFIER = chat.rocket.ios;
PRODUCT_BUNDLE_IDENTIFIER = chat.rocket.ios11;
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to remove this change.

@@ -2565,11 +2577,11 @@
CODE_SIGN_ENTITLEMENTS = Rocket.Chat/Rocket.Chat.entitlements;
CODE_SIGN_IDENTITY = "iPhone Developer";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer";
DEVELOPMENT_TEAM = S6UPZG7ZR3;
DEVELOPMENT_TEAM = MJHBLUH3VK;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this change?

INFOPLIST_FILE = Rocket.Chat/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 10.0;
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks";
PRODUCT_BUNDLE_IDENTIFIER = chat.rocket.ios;
PRODUCT_BUNDLE_IDENTIFIER = chat.rocket.ios11;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this change?


"alert.audio_message.error.title" = "Ooops!";
"alert.audio_message.error.message" = "An error has occurred. Please, record your message again.";

Copy link
Contributor

Choose a reason for hiding this comment

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

We've to add this strings to the other localizable files (in English is fine if you don't know the other languages). Is just to keep it in sync.

- Minor project tweaks
- Localization config (translation to german, polish and czech pending_
@luismachado
Copy link
Contributor Author

@rafaelks Okay, just addressed your remarks. Regarding the translation, I was going to add the PT version but it looks like it was already there :P

Therefore I just added the lines for the other 3 but left a TODO tag near each one, as I don't trust google translate with longer phrases...

@rafaelks
Copy link
Contributor

Looking good, but I noticed something curious... always, the first time I send the audio, it doesn't work, stays with 00:00 after the upload. Can you also reproduce this issue?

@luismachado
Copy link
Contributor Author

Yeah, also just noticed that. It's also happening in @falcaoaugustos branch.
It looks to always be the first recording per launch (even if you don't send the 1st one the 2nd one will be okay).

Going to try to find out what's happening.

- Fixed bug where first audio recording per each app launch wasn't being made
- Dismiss keyboard when recording button is pressed
@luismachado
Copy link
Contributor Author

Okay, the issue seems now seems to be fixed.

Would you care to check it on your end @rafaelks ? Thanks.

@rafaelks
Copy link
Contributor

Thank you @luismachado & @falcaoaugustos!!! 🚀

@rafaelks rafaelks merged commit fe015ce into RocketChat:develop Dec 13, 2017
@falcaoaugustos
Copy link
Contributor

Thank you very much @luismachado . I had a few problems fixing it but I will read your code in order to learn how to do so. I must say that I am still studying this case.
Thank you too, @rafaelks . By the way, I will make more contributions as soon as I solve my money issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants