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.

Sign up for GitHub

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

Jump to bottom

Feature: Take photos and videos and record voice memos #961

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Feature: Take photos and videos and record voice memos #961

wants to merge 16 commits into from

Conversation

KhaledNjim
Copy link
Collaborator

@KhaledNjim KhaledNjim commented Jun 10, 2024 •
edited
Loading

User story

Demo (android 34)

34.webm

Demo (android 33)

Screen_recording_20240603_211703.webm

Demo (android 28)

28.webm

Demo (android 27)

27.webm

Camera permission demo

permission_cam.webm

Audio recording permission demo

recper.webm

Open settings when permission is permantly denied

openSettings.webm

@KhaledNjim KhaledNjim requested a review from hoangdat June 10, 2024 04:49
@hoangdat
Copy link
Member

What is 1267? Can you describe about this issue in ADR or in github ticket, and reference commits to it?

@hoangdat
Copy link
Member

hoangdat commented Jun 10, 2024 •
edited by KhaledNjim
Loading

  • demo for handling permission case also
  • demo for iOS too

hoangdat
hoangdat reviewed Jun 10, 2024
View reviewed changes
ios/Runner/Info.plist Outdated Show resolved Hide resolved
hoangdat
hoangdat reviewed Jun 10, 2024
View reviewed changes
ios/Runner/Info.plist Outdated Show resolved Hide resolved
@KhaledNjim
Copy link
Collaborator Author

  • demo for handling permission case also
  • demo for iOS too

idont have mac for ios demo

@KhaledNjim
Copy link
Collaborator Author

What is 1267? Can you describe about this issue in ADR or in github ticket, and reference commits to it?

added adr

@hoangdat
Copy link
Member

what happen if you reject permission?

hoangdat
hoangdat reviewed Jun 12, 2024
View reviewed changes
lib/presentation/util/audio_recorder.dart Outdated
Comment on lines 61 to 68
await recorderController.stop().then((value) {
if (value != null) {
var recordedFile = File(value);
file = FileInfo(recordedFile.path.split('/').last,
'${recordedFile.parent.path}/', recordedFile.lengthSync());
pickedFiles.add(file);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

what happens if error occur here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added error handling now it will display a toast and return to the previous screen

hoangdat
hoangdat reviewed Jun 12, 2024
View reviewed changes
pubspec.yaml Show resolved Hide resolved
hoangdat
hoangdat reviewed Jun 12, 2024
View reviewed changes
lib/presentation/widget/record_audio/record_audio_viewmodel.dart Outdated
Comment on lines 73 to 93
stopwatch.stop();
audioRecorder.stopRecordingAndSave().then((result) {
result.fold((failure) {
store.dispatch(StopRecording());
store.dispatch(UploadFileAction(Left(failure)));
}, (success) async {
store.dispatch(StopRecording());
_appNavigation.popBack();
store.dispatch(UploadFileAction(Right(success)));
var uploadArgs = UploadFileArguments(success.file);
if (uploadFileArguments != null) {
uploadArgs.shareType = uploadFileArguments!.shareType;
uploadArgs.workGroupDocumentUploadInfo =
uploadFileArguments!.workGroupDocumentUploadInfo;
}
await _appNavigation.push(RoutePaths.uploadDocumentRoute,
arguments: uploadArgs);
});
});
Copy link
Member

Choose a reason for hiding this comment

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

what happen if error throw here?

hoangdat
hoangdat reviewed Jun 12, 2024
View reviewed changes
lib/presentation/widget/record_audio/record_audio_viewmodel.dart Outdated
Comment on lines 67 to 69
store.dispatch(PauseRecording());
stopwatch.stop();
audioRecorder.pauseRecording();
Copy link
Member

Choose a reason for hiding this comment

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

idem

hoangdat
hoangdat reviewed Jun 12, 2024
View reviewed changes
lib/presentation/widget/myspace/my_space_viewmodel.dart Outdated
Copy link
Member

Choose a reason for hiding this comment

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

a lot of format make the review hard

@hoangdat
Copy link
Member

hoangdat commented Jun 12, 2024 •
edited
Loading

please use git fixup, dont use force push. It takes a lot of time to review the old one.
I will refuse to continue if you force push on this big changes.

@KhaledNjim
Copy link
Collaborator Author

what happen if you reject permission?

display a toast and return to the previous screen

@KhaledNjim KhaledNjim requested a review from hoangdat June 26, 2024 10:15
@KhaledNjim
Added ui elements for taking photos
1e378fd
@KhaledNjim
Added presentation layer for taking a photo or video
3ed1a0c
@KhaledNjim
Added redux/domain and presentation layer for audio recording
c7fc872
@KhaledNjim
Updated ui for context menu in sharedSpace and mySpace
b6fb06a
@KhaledNjim
Updated camera and microphone permission for ios
7e322ff
@KhaledNjim
Updated icons for file upload context menu in shared_space and my_space
4c6a3fb
@KhaledNjim
fixup! Added redux/domain and presentation layer for audio recording
bdd07da
@KhaledNjim
Changed redux action for camera picker
75d9ba4
@KhaledNjim
added adr for audio recording and camera usage
ac23392
@KhaledNjim
fixup! Added presentation layer for taking a photo or video
e8cd889
@KhaledNjim
fixup! fixup! Added redux/domain and presentation layer for audio rec…
129d930 
…ording
@KhaledNjim
fixup! Added redux/domain and presentation layer for audio recording
591d115
@KhaledNjim
fixup! Added presentation layer for taking a photo or video
6b8165d
@KhaledNjim
Copy link
Collaborator Author

Hi @KhaledNjim , please rebase with master branch and force push. Because we had some change in CICD. Thanks

done

@hoangdat
Copy link
Member

hoangdat commented Jul 2, 2024 •
edited by KhaledNjim
Loading

  • problem when select audio but not record anything before
Screenrecorder-2024-07-02-20-07-37-863.mp4

@hoangdat
Copy link
Member

hoangdat commented Jul 2, 2024 •
edited by KhaledNjim
Loading

  • problem when record video, play it, but can not to confirm
    Reproduce steps:
  1. click Camera
  2. long press to record video
  3. Play video
  4. Can not to Confirm
Screenrecorder-2024-07-02-20-06-46-612.mp4

@hoangdat
Copy link
Member

hoangdat commented Jul 2, 2024 •
edited by KhaledNjim
Loading

  • time of recording is not update well
    Reproduce:
  1. Record audio
  2. back
  3. Plus -> Record audio
  4. do it again
  5. Observe
Screenrecorder-2024-07-02-20-22-10-110.mp4

hoangdat
hoangdat requested changes Jul 2, 2024
View reviewed changes
Copy link
Member

@hoangdat hoangdat left a comment

Choose a reason for hiding this comment

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

critical issues

@KhaledNjim
Copy link
Collaborator Author

  • problem when record video, play it, but can not to confirm
    Reproduce steps:
  1. click Camera
  2. long press to record video
  3. Play video
  4. Can not to Confirm

Screenrecorder-2024-07-02-20-06-46-612.mp4

fixed

probvideo.webm

@KhaledNjim
Copy link
Collaborator Author

  • time of recording is not update well
    Reproduce:
  1. Record audio
  2. back
  3. Plus -> Record audio
  4. do it again
  5. Observe

Screenrecorder-2024-07-02-20-22-10-110.mp4

Fixed

counter.webm

@KhaledNjim
Copy link
Collaborator Author

  • problem when select audio but not record anything before

Screenrecorder-2024-07-02-20-07-37-863.mp4

Fixed

issue3.webm

@KhaledNjim KhaledNjim requested a review from hoangdat July 5, 2024 01:56
@hoangdat
Copy link
Member

hoangdat commented Jul 16, 2024 •
edited
Loading

  • should show toast to ask user when he click back button during recording.
  • should handle recording/video state in case of phone comming during recording or video

@hoangdat
Copy link
Member

  • should ask user when user try to click on back button in confirmation screen of video

hoangdat
hoangdat reviewed Jul 16, 2024
View reviewed changes
android/app/build.gradle
Copy link
Member

Choose a reason for hiding this comment

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

IMO, we should change compileSdkVersion to 34 to make video works

@hoangdat
Copy link
Member

hoangdat commented Jul 16, 2024 •
edited by KhaledNjim
Loading

  • when trying to select people in suggestion, list show error
  1. record video/audio
  2. share
  3. typing big
  4. select an item

Screenshot_2024-07-16-09-13-06-568_com linagora android linshare

@KhaledNjim
Copy link
Collaborator Author

KhaledNjim commented Jul 16, 2024 •
edited
Loading

  • when trying to select people in suggestion, list show error
  1. record video/audio
  2. share
  3. typing big
  4. select an item

Screenshot_2024-07-16-09-13-06-568_com linagora android linshare

Unable to reproduce this one, please verify you are running the app with no-sound-null-safety

Demo:

linshareSharingBug.webm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoangdat hoangdat hoangdat requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KhaledNjim @hoangdat