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

added a one-step image/video upload flow #62

Closed
wants to merge 7 commits into from
Closed

added a one-step image/video upload flow #62

wants to merge 7 commits into from

Conversation

shubhamtewari
Copy link
Contributor

@shubhamtewari shubhamtewari commented Sep 22, 2019

  1. a new one step image/video upload flow has been added (open the image chooser, select multiple images/videos and click open), required files have been modified
  2. the MobicomAttachmentSelectorActivity has be modified to better seperate and UI and the controlling fucntions
  3. The file async task has been improved to be more generic and moved to a seperate file
  4. the KmAttachmnetsControllr has been created to seperate the code( refer to point 2) and the functions common between the new flow and the old flow
  5. comments have been added where ever i found difficulty in understanding in what the function did (only for the attachment upload parts)

@shubhamtewari
Copy link
Contributor Author

By takephoto, it means any photo or is it for taken from camera?

it means photo taken from camera

@devashishmamgain
Copy link
Contributor

By takephoto, it means any photo or is it for taken from camera?

it means photo taken from camera

then we can rename fromTakePhoto to fromCamera

*/
public interface PrePostUIMethods {
void preTaskUIMethod();
void postTaskUIMethod(boolean b, File file);
Copy link
Contributor

Choose a reason for hiding this comment

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

@shubhamtewari please use proper variable names and not a, b, c, d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

Copy link
Contributor

Choose a reason for hiding this comment

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

still showing boolean b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be changed now

@shubhamtewari
Copy link
Contributor Author

Sir regarding the fromCamera and fromMultiSelectActivity.

  1. The methods processCamera() and processMultiSelectActivity() inside them have if statements that check for permissions from the user. If the permissions aren't present, the statements inside are not executed and instead permission is asked from the user.
  2. these permissions are handled in the onRequestPermissionResult() call-back in ConversationActivity.java
  3. Inside this call-back, the variables fromCamera and fromMultiSelectActivity are used.
  4. they are important and removing them will break the code.

WHY can't they be set to true inside the methods themselves?

  • beacause the processCamera() and procesMultiSelectGallery methods are used in other places too, where the setting of these variable to true isnt needed.
  • also having them outside is better for understanding since the method logic doesn't require these variables, they are external to the working of those methods

@reytum reytum closed this Oct 1, 2019
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.

None yet

3 participants