-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Added direct upload from video camera #12995
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution.
I try to upload video directly on Android 14 emulator, after completing video shooting upload didn't work. The video file didn't upload.
After my second attempt, the app is crashed. Logcat didn't show any crash log for nextcloud package and general android
Could you test via Android 14 emulator and share your findings with us?
Thanks for testing, thats what i meant with "It seems that the videointent doesn't work on emulator?", I tried countless times using the videointent on an emulator and it crashes or exits. I currently have no idea why this happens. On a hardware phone (Pixel 6a) it just works... (i was unsure if it was maybe just my underpowered laptop, but this ensures the bug) I'll try some tests with only video intents (outside of the nextcloup app) on emulator to check for the issue. |
Hey, I'm sorry, but I currently don't have the time (and motivation) to further check on this issue in the emulator. Maybe someone else finds the problem, or just discard the PR? |
I'll check it. Thank you for your contribution; it's wonderful to see support from the community 👍 |
For me, the PR seems to work on real device (One Plus running Android 12). |
What do you think of not having 2 different "upload from camera" in the Dialog Fragment but just one and then showing the user the dialog that's currently shown when accepting the permissions with the options video and image? I think the DialogFragment is already a little crowded, and it would make the experience a little more consistent since the dialog is currently only shown once regardless if "upload from camera" or "upload from video camera" is clicked... @alperozturk96 @timedin-de What do you think? |
Yep I could do it this way if you agree on it. I chose the current approach because of the comment in the issue |
@timedin-de thank you! I guess the best persons to decide on that is the design team @nextcloud/designers Option 1 for adding a "capture video" implementation would look something like this: Pro: One click video capture Option 2 would look something like this: Pro: Consistent, less crowded |
Honestly I dont unterstand what is the difference between upload from camera and upload from video camera? |
On Android when a camera gets started it needs to be specified if it should record a video or take a picture (you cant decide/change it inside of the camera preview). So "video camera" starts in video-recording-mode, "camera" in photo-mode |
I see. Then maybe rename the entries to |
@szaimen Yes good point. But the question is if we should even use 2 separate buttons (video and image) when clicking on the plus in the app. Or if we combine it in one button and the ask the user in a dialog if he wants video or image (prefered option for me and Alper). Take a look at my message for the pro and cons in my opinion: #12995 (comment) |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
I would probably indeed combine these two things into one option: Then if clicking on that, indeed open a popup and ask this question but put the photo button to the right as it is more likely that you want to take a photo imho. |
So change the text for the file-action, instead of leaving it "Upload from camera"? |
@timedin-de @szaimen I would leave it as it was. "Take photo or video and upload" feels a little long, an "Upload from camera" is already expressive enough in my opinion? |
I think "Take photo or video and upload" is better but if it is too long "Upload from camera" should work as well |
Signed-off-by: TimedIn <[email protected]>
Ok I left it on "Upload from camera". For implementation I moved the permissons-code from |
@@ -443,7 +444,7 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis | |||
// If request is cancelled, result arrays are empty. | |||
if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { | |||
// permission was granted | |||
getFileOperationsHelper().uploadFromCamera(this, FileDisplayActivity.REQUEST_CODE__UPLOAD_FROM_CAMERA); | |||
getOCFileListFragmentFromFile().directCameraUpload(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displaying the dialog in OCFileListFragment
|
||
if (!file.renameTo(renamedFile)) { | ||
DisplayUtils.showSnackMessage(getActivity(), "Fail to upload taken image!"); | ||
DisplayUtils.showSnackMessage(getActivity(), R.string.error_uploading_direct_camera_upload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not regarding the video-upload itself, small improvement
viewThemeUtils.platform.colorImageView(binding.menuIconUploadFiles, ColorRole.PRIMARY); | ||
viewThemeUtils.platform.colorImageView(binding.menuIconUploadFromApp, ColorRole.PRIMARY); | ||
viewThemeUtils.platform.colorImageView(binding.menuIconDirectCameraUpload, ColorRole.PRIMARY); | ||
viewThemeUtils.platform.colorImageView(binding.menuIconScanDocUpload, ColorRole.PRIMARY); | ||
viewThemeUtils.platform.colorImageView(binding.menuIconMkdir, ColorRole.PRIMARY); | ||
viewThemeUtils.platform.colorImageView(binding.menuIconAddFolderInfo, ColorRole.PRIMARY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not regarding video upload. Fixing Deprecation
Regarding feature request #7358
Adding Option to directly upload a video
If camera permissions are not already granted, it will ask if for video or image after granting (its seems not not to be possible to get the camera-action after the permission request)
It seems that the videointent doesn't work on emulator?
Tested successfull with a real GooglePixel 6a
Tests written, or not not needed