Pictures can be upload directly clicking them from the device's camera. #2097

shashvat-kedia · 2018-01-24T15:06:14Z

Fix to Issue #2027

BUGS & IMPROVEMENTS

(1) File name of pics Pictures can be upload directly clicking them from the device's camera. #2097 (comment) [FIXED]
(2) Option switched from copy to move Pictures can be upload directly clicking them from the device's camera. #2097 (comment) [FIXED]
(3) Wording issues [FIXED]

davigonz · 2018-01-24T16:49:03Z

Hi @SD1998 , thank you for opening this again, using a new branch for a new feature is always the best approach 👍

I've seen that the commit of the android-library (7c7cad7) is not the proper one, please update it to the latest one (0e7fd8e).

From your Android app directory:

cd owncloud-android-library
git checkout 0e7fd8ee8fb1ad082acba8911412571b206ee1f0
cd ..
git add owncloud-android-library
git commit ...
git push ...

shashvat-kedia · 2018-01-24T16:57:34Z

@davigonz Is it the latest one now? (I have just changed it)

davigonz · 2018-01-24T16:59:08Z

Is it the latest one now? (I have just changed it)

@SD1998 Yes, the library change has just disappeared from the Files changed tab, thanks 👏

shashvat-kedia · 2018-01-25T06:32:32Z

@davigonz Has the QA started?

davigonz · 2018-01-25T13:21:31Z

res/values/strings.xml

@@ -82,6 +82,10 @@
    <string name="file_list_file">file</string>
    <string name="file_list_files">files</string>
    <string name="filedetails_select_file">Tap on a file to display additional information.</string>
+    <string name="upload_owncloud"> Upload to OwnCloud </string>
+    <string name="upload_bottom_sheet_displayed"> Upload Bottom Sheet Displayed </string>


This resource is not being used, please delete it

Actually I am using upload_owncloud

I meant upload_bottom_sheet_displayed, which you've already deleted.

There's still some space at the beginning and end of Upload to ownCloud in upload_owncloud resource. Do you really need it? If so, please use margin or padding in the xml layout.

davigonz · 2018-01-25T13:21:45Z

res/values/strings.xml

@@ -82,6 +82,10 @@
    <string name="file_list_file">file</string>
    <string name="file_list_files">files</string>
    <string name="filedetails_select_file">Tap on a file to display additional information.</string>
+    <string name="upload_owncloud"> Upload to OwnCloud </string>
+    <string name="upload_bottom_sheet_displayed"> Upload Bottom Sheet Displayed </string>
+    <string name="upload_bottom_sheet_collapsed"> Upload Bottom Sheet Collapsed </string>


Same here, delete it

davigonz · 2018-01-25T13:22:52Z

res/values/strings.xml

@@ -176,6 +180,7 @@
    <string name="foreign_files_remote_text">"Remote: %1$s"</string>
    <string name="upload_query_move_foreign_files">There is not enough space to copy the selected files into the %1$s folder. Would you like to move them instead? </string>
    <string name="pass_code_enter_pass_code">Please insert your passcode</string>
+    <string name="bottom_sheets_layout_behaviour"> android.support.design.widget.BottomSheetBehavior </string>


Not used, can be deleted

davigonz · 2018-01-25T13:24:10Z

res/values/strings.xml

@@ -205,6 +210,9 @@
    <string name="media_rewind_description">Rewind button</string>
    <string name="media_play_pause_description">Play or pause button</string>
    <string name="media_forward_description">Fast forward button</string>
+    <string name="button_text_camera"> Camera </string>


Why are you using spaces before and after the value of this resource? If you want to add a margin or padding, please include it in the xml file instead of here.

Are you using button_text_camera resource? Cannot find its use.

davigonz · 2018-01-25T13:24:24Z

res/values/strings.xml

@@ -205,6 +210,9 @@
    <string name="media_rewind_description">Rewind button</string>
    <string name="media_play_pause_description">Play or pause button</string>
    <string name="media_forward_description">Fast forward button</string>
+    <string name="button_text_camera"> Camera </string>
+    <string name="upload_from_camera_title"> Upload From Camera </string>


Same here, there are several ones

davigonz · 2018-01-25T13:24:47Z

res/values/strings.xml

@@ -205,6 +210,9 @@
    <string name="media_rewind_description">Rewind button</string>
    <string name="media_play_pause_description">Play or pause button</string>
    <string name="media_forward_description">Fast forward button</string>
+    <string name="button_text_camera"> Camera </string>
+    <string name="upload_from_camera_title"> Upload From Camera </string>
+    <string name="capture_button_text"> Capture </string>


capture_button_text seems not to be used either.

Yes I just saw that. I used it when I was not using bottom sheets and instead using a FAB.

davigonz · 2018-01-25T13:32:43Z

src/com/owncloud/android/ui/activity/UploadFilesActivity.java


                SharedPreferences.Editor appPreferencesEditor = PreferenceManager
                        .getDefaultSharedPreferences(getApplicationContext()).edit();

-
-                if (mRadioBtnMoveFiles.isChecked()){
+                if(requestCode == FileDisplayActivity.REQUEST_CODE__UPLOAD_FROM_CAMERA){


You are using the condition if(requestCode == FileDisplayActivity.REQUEST_CODE__UPLOAD_FROM_CAMERA){ twice here to set several params, is not possible to use just one and unify all the code?

Yes I have done that

davigonz · 2018-01-25T13:36:01Z

Has the QA started?

@SD1998 No, I've just passed what I think is going to be last code review. After that, @jesmrec will start QA.

shashvat-kedia · 2018-01-25T13:44:56Z

@davigonz I will work on this I remember using some of the string resources in Log_OC.d() I will check them again. If I am not using them then I will remove them.
Regarding the last one I think one way to unify the code will be to create another class that extends AsyncTask and execute it when the user selects the upload from camera option from the bottom sheet. But I did not do that in the first place because the new AsyncTask will be very much similar to CheckAvailableSpaceTask class (the part in doInBackground() and onPostExecute()) which will result in much more code duplication hence I used the current approach. Should I add another AsyncTask and not use the if statements?

davigonz · 2018-01-25T14:28:10Z

Regarding the last one I think one way to unify the code will be to create another class that extends AsyncTask and execute it when the user selects the upload from camera option from the bottom sheet. But I did not do that in the first place because the new AsyncTask will be very much similar to CheckAvailableSpaceTask class (the part in doInBackground() and onPostExecute()) which will result in much more code duplication hence I used the current approach. Should I add another AsyncTask and not use the if statements?

@SD1998 What I meant is something easier, like this:

shashvat-kedia · 2018-01-25T14:37:40Z

@davigonz Ok I will work on this.

shashvat-kedia · 2018-01-29T05:45:23Z

@davigonz Are there any more changes required?

davigonz · 2018-01-29T08:01:52Z

Are there any more changes required?

@SD1998 thanks for applying the changes, have a look at the next three comments:

Pictures can be upload directly clicking them from the device's camera. #2097 (comment)
Pictures can be upload directly clicking them from the device's camera. #2097 (comment)
Pictures can be upload directly clicking them from the device's camera. #2097 (comment)

shashvat-kedia · 2018-01-29T14:59:34Z

I have done all the changes.

davigonz · 2018-01-30T08:37:45Z

@SD1998 Thank you, it looks good to me, changes approved, ready to pass QA @jesmrec

jesmrec · 2018-02-02T11:00:44Z

Starting QA here...

jesmrec · 2018-02-02T11:04:15Z

IMPROVEMENT (1) [DONE]

When a pic is taken, the name it is upload with is the epoch time of the moment. This is not incorrect, of course, but maybe is more clear to set the calendar date as filename, for example: 2018-02-02-1159.JPG or similar, like the style used for most of the cameras.

what do you think?

shashvat-kedia · 2018-02-02T11:18:54Z

@jesmrec I think epoch time will be better as it is possible to take two picture in a minute and upload them. Thus resulting in naming conflicts. For ex. If two pictures are taken at 11:59 then both of them will be named 2018-02-02-1159.JPG.

jesmrec · 2018-02-02T11:52:02Z

it can be set always the second fraction, i mean 2018-02-02-115934.JPG what do you think @davigonz ?

davigonz · 2018-02-02T12:16:45Z

it can be set always the second fraction, i mean 2018-02-02-115934.JPG what do you think @davigonz ?

@jesmrec @SD1998 I would use something like IMG_20180131_185750.jpg , which is the current format used by the default Android camera app to save the taken pictures and the users are used to it.

jesmrec · 2018-02-02T12:18:20Z

BUG (2) [FIXED]

Steps to reproduce:

Install the app
Try to upload any file (not from camera). The option copy/move (bottom of the view) is set to copy by default
Go back to files view, open FAB and upload file from camera.
Take a pic and accept it -> it is uploaded correctly
Try to upload again a file (not from camera)

Current: Option has switched from copy to move
Expected: Option remains as copy

Tested with Nexus5 v6

shashvat-kedia · 2018-02-02T12:18:59Z

@davigonz @jesmrec This is the current naming format in which the pictures are getting uploaded.

shashvat-kedia · 2018-02-02T12:21:18Z

@jesmrec Bug is beacuse I am moving the files that are uploaded directly from the camera. I will fix this now.

davigonz · 2018-02-06T15:12:45Z

In this new layout we can make the files and the upload from camera option to be side by side rather than being one below another.

@SD1998 Even with the layout you propose, If we use PEEK_HEIGHT_AUTO, I think we would continue having the problem described in #2097 (comment) and if we do not use PEEK_HEIGHT_AUTO, it would reappear the problem described in #2097 (comment).

There's likely to be a common solution to show all the options both in landscape and portrait modes.

The problem with bottom sheets in landscape mode seems to be known, have a look at https://stackoverflow.com/questions/41591733/bottom-sheet-landscape-issue , it may be useful for you.

jesmrec · 2018-02-07T09:02:39Z

@SD1998 do you think is feasible to fix that?

davigonz · 2018-02-08T10:05:13Z

src/com/owncloud/android/ui/fragment/OCFileListFragment.java

@@ -100,6 +104,11 @@
    private OCFile mFile = null;
    private FileListListAdapter mAdapter;

+    private LinearLayout uploadFilesLinearLayout;


These four variables do not need to be declared globally, you can declare it directly in registerFabUploadListeners method, where are going to be used.

davigonz · 2018-02-08T10:15:49Z

src/com/owncloud/android/ui/fragment/OCFileListFragment.java

+                });
+                uploadToTextView.setText(String.format(getResources().getString(R.string.upload_to),getResources().getString(R.string.app_name)));
+                uploadBottomSheetBehavior = BottomSheetBehavior.from((View) uploadBottomSheet.getParent());
+                uploadBottomSheetBehavior.setPeekHeight(-1);


I've managed to fix #2097 (comment) and #2097 (comment) following the next steps:

Delete line uploadBottomSheetBehavior.setPeekHeight(-1);

Include the next code:

dialog.setOnShowListener(new DialogInterface.OnShowListener() { @Override public void onShow(DialogInterface dialog) { uploadBottomSheetBehavior.setPeekHeight(uploadBottomSheet.getMeasuredHeight()); } });

With this code, we wait till the dialog is shown, measure its height and use it to set the height of the bottom sheet. I've tested it with different devices (portrait & landscape) and seems to be working properly.

@davigonz Thanks for the help. I trying to find the value of the height of bottom sheet using ViewTreeObserver class but it did not work out. I will implement the changes.

davigonz · 2018-02-08T10:20:58Z

Hi @SD1998, I've managed to fix the problem with the bottom sheet height to give you a hand, please have a look at the changes I requested above.

Besides, please update the camera icon for xxxhdpi as I described in #2097 (comment).

After that, @jesmrec will perform the last tests, we will merge and continue with other interesting features.

Thanks

shashvat-kedia · 2018-02-08T10:41:32Z

@davigonz I have made all the changes and added the xxxhdpi icon

davigonz · 2018-02-08T11:35:48Z

src/com/owncloud/android/ui/fragment/OCFileListFragment.java

-                uploadBottomSheetBehavior = BottomSheetBehavior.from((View) uploadBottomSheet.getParent());
-                uploadBottomSheetBehavior.setPeekHeight(-1);
+                final BottomSheetBehavior uploadBottomSheetBehavior = BottomSheetBehavior.from((View) uploadBottomSheet.getParent());
+                uploadBottomSheetBehavior.setState(BottomSheetBehavior.STATE_EXPANDED);


I think you don't need this line, the method below that would be enough to fix the problem

@davigonz Yes I forgot to remove it. Sorry I will remove it now.

davigonz · 2018-02-08T11:54:41Z

Thanks @SD1998 , let's start with the last QA round @jesmrec

jesmrec · 2018-02-09T08:01:06Z

The icon size is correct and now the bottom sheet appears and vanish correctly (checked with three devices in different screen sizes).

Finally, we can approve this

@SD1998 please rebase first, then the branch will be merged

davigonz · 2018-02-09T09:40:56Z

Thanks for the QA @jesmrec , I will merge when @SD1998 rebases the branch with master

shashvat-kedia · 2018-02-09T09:44:53Z

@davigonz I have done the rebase.

davigonz · 2018-02-09T10:04:56Z

@SD1998 @jesmrec Merged, this feature will be included in the next 2.7.0 release, great work 👍

shashvat-kedia · 2018-02-09T10:16:17Z

@davigonz @jesmrec Thanks.

davigonz suggested changes Jan 25, 2018

View reviewed changes

shashvat-kedia force-pushed the Fix-2027-Rebase branch 2 times, most recently from 7d1fd1f to d481dd1 Compare January 27, 2018 07:19

shashvat-kedia force-pushed the Fix-2027-Rebase branch from d481dd1 to ebad0fc Compare January 29, 2018 05:46

davigonz approved these changes Jan 30, 2018

View reviewed changes

shashvat-kedia force-pushed the Fix-2027-Rebase branch 2 times, most recently from 8e1800e to 27ef6a1 Compare February 1, 2018 13:18

shashvat-kedia mentioned this pull request Feb 1, 2018

Pattern Lock Added #2083

Merged

3 tasks

jesmrec mentioned this pull request Feb 2, 2018

[PR 2097] Upload from camera owncloud/QA#513

Closed

shashvat-kedia force-pushed the Fix-2027-Rebase branch from 27ef6a1 to 72307ee Compare February 2, 2018 11:20

davigonz suggested changes Feb 8, 2018

View reviewed changes

shashvat-kedia force-pushed the Fix-2027-Rebase branch from c00baa6 to 7aa392d Compare February 8, 2018 10:37

davigonz suggested changes Feb 8, 2018

View reviewed changes

davigonz approved these changes Feb 8, 2018

View reviewed changes

jesmrec added the Approved by qa label Feb 9, 2018

shashvat-kedia added 13 commits February 9, 2018 15:13

Feature Added

42664bc

Deleted String Added

6ace224

owncloud-android-library added

4784e30

Code Review Fixes

ffafb5c

Code Review Fixes

c52812d

QA fixes

272f0c4

Timestamp bug fixed

8815250

Wording issue fixed

a6b554f

Code Review Fixes

b46e9ab

Bottom sheet bug fixed

c8deef5

Upload bottom sheet bug fixed

46b9429

Camera xxxhdpi icon added

19ecc50

Code review fixes

f13d2aa

shashvat-kedia force-pushed the Fix-2027-Rebase branch from 872cf00 to f13d2aa Compare February 9, 2018 09:44

davigonz merged commit 14b4379 into owncloud:master Feb 9, 2018

Pictures can be upload directly clicking them from the device's camera. #2097

Pictures can be upload directly clicking them from the device's camera. #2097

Conversation

shashvat-kedia commented Jan 24, 2018 • edited by jesmrec Loading

davigonz commented Jan 24, 2018

shashvat-kedia commented Jan 24, 2018

davigonz commented Jan 24, 2018

shashvat-kedia commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz commented Jan 25, 2018

shashvat-kedia commented Jan 25, 2018

davigonz commented Jan 25, 2018 • edited Loading

shashvat-kedia commented Jan 25, 2018

shashvat-kedia commented Jan 29, 2018

davigonz commented Jan 29, 2018 • edited Loading

shashvat-kedia commented Jan 29, 2018 • edited Loading

davigonz commented Jan 30, 2018 • edited Loading

jesmrec commented Feb 2, 2018

jesmrec commented Feb 2, 2018 • edited Loading

shashvat-kedia commented Feb 2, 2018 • edited Loading

jesmrec commented Feb 2, 2018

davigonz commented Feb 2, 2018

jesmrec commented Feb 2, 2018 • edited Loading

shashvat-kedia commented Feb 2, 2018

shashvat-kedia commented Feb 2, 2018

davigonz commented Feb 6, 2018 • edited Loading

jesmrec commented Feb 7, 2018

Choose a reason for hiding this comment

davigonz Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz commented Feb 8, 2018

shashvat-kedia commented Feb 8, 2018

davigonz Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davigonz commented Feb 8, 2018

jesmrec commented Feb 9, 2018 • edited Loading

davigonz commented Feb 9, 2018

shashvat-kedia commented Feb 9, 2018

davigonz commented Feb 9, 2018

shashvat-kedia commented Feb 9, 2018

shashvat-kedia commented Jan 24, 2018 •

edited by jesmrec

Loading

davigonz Jan 29, 2018 •

edited

Loading

davigonz Jan 29, 2018 •

edited

Loading

davigonz commented Jan 25, 2018 •

edited

Loading

davigonz commented Jan 29, 2018 •

edited

Loading

shashvat-kedia commented Jan 29, 2018 •

edited

Loading

davigonz commented Jan 30, 2018 •

edited

Loading

jesmrec commented Feb 2, 2018 •

edited

Loading

shashvat-kedia commented Feb 2, 2018 •

edited

Loading

jesmrec commented Feb 2, 2018 •

edited

Loading

davigonz commented Feb 6, 2018 •

edited

Loading

davigonz Feb 8, 2018 •

edited

Loading

davigonz Feb 8, 2018 •

edited

Loading

jesmrec commented Feb 9, 2018 •

edited

Loading