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

Picking files #53

Merged
merged 72 commits into from
Mar 1, 2017
Merged

Picking files #53

merged 72 commits into from
Mar 1, 2017

Conversation

JamesMcIntosh
Copy link

Hi,

I have been investigating your library and found your implementation using RxJava to be very useful for retrieving images from the camera and gallery.

What I also wanted to do was to be able to request arbitrary non-image files e.g. PDF. Instead of bringing in yet another library I made these changes.

I realise that this functionality steps outside of the project description "RxJava extension for Android to access camera and gallery to take images." so let me know what you think about incorporating the idea.

I have also made the changes for the 1.x branch which I can create another pull-request.
https://github.com/Agrimap/RxPaparazzo/tree/master-agrimap

A further cleanup can be done to remove Gallery / PickImage / PickImages and just use the Files / PickFile / PickFiles and mimeType filtering to accomplish the same thing. I was just not sure how much of Gallery / PickImage(s) you would want to keep for backwards compatibility.

Kind regards
James McIntosh

@JamesMcIntosh JamesMcIntosh changed the title Abstract picking files out of picking images Picking files Dec 20, 2016
@miguelbcr
Copy link
Owner

Hi @JamesMcIntosh ,
Let me study your approach in these days (a busy days) and I'll tell you something.
Thanks!

@JamesMcIntosh
Copy link
Author

Looks like I need to do a few more things to get this working better, I'll see if I can address them before the new year.

  1. There is no control of final destination filename
  2. The filename of the selected file is not available
  3. You may wish to save a file is a given destination but also know about the original filename
  4. CropImage and SaveImage should only happen when the source is an image
  5. Cropping / resizing of the selected image defaults to the screen resolution of the device where maintaining the original size was confusing (can use OriginalSize class as config option)

@JamesMcIntosh
Copy link
Author

  1. I decided that setting the destination filename can be handled later.
  2. I've made the changes so that we can pass through the original filename.
  3. As per When using gallery, it opens explorer to pick image instead of gallery #1 & When crop option is not selected then the original image is deleted. #2
  4. Crop image and save image have don't cause any problems being in the pipeline.
  5. Default size can remain same, making the change using config is very easy. It may be confusing when someone is using the file picker and does not intend the images to be resized. Reading the whole README does spell it out and keeping it consistent between the camera/gallery/file picker is sensible.

@JamesMcIntosh
Copy link
Author

JamesMcIntosh commented Jan 15, 2017

Hi @miguelbcr

Do you have any time to review the current progress?

Why are the images submitted to the media scanner during the save action?

During the save action the source file is deleted, this makes some sense if you have just resized/cropped the file, can you see any reason not to delete it?

Additional work:

  1. Check changes to default file extension in ImageUtils when not provided by source image. (done)
  2. Pass through document title / description from the MediaStore where different from file name. (done)
  3. SecurityException / permission issue when selecting second file from Google Drive (works the first time only)
  4. Specify desired mime type of files for file picker using configuration. (done)
  5. The image resize/crop/saving pipeline which occurs after you have selected the file(s) is similar/identical between gallery/camera/files, look at refactoring this into a reusable function. (done - removed gallery and image pickers - done using mime type instead)

Many thanks
James

@miguelbcr
Copy link
Owner

Hi @JamesMcIntosh,
Thanks for your work.
sorry for the delay, I've been busy, but I'll try to check your changes during this week.

About your questions:

  • Why are the images submitted to the media scanner during the save action?
    In order to show to the user the new gallery folder with its app images, but maybe it could be a user choice ...

  • During the save action the source file is deleted, this makes some sense if you have just resized/cropped the file, can you see any reason not to delete it?
    It was because any image was always copied (as a temp image), even if it was the original image, so the deleted image was the temp image. Maybe with your new approach it wouldn't be neccessary anymore...

@Yazon2006
Copy link

@JamesMcIntosh Great job! I've just thought about changes that you did. Are there possibility to add in dependencies aar from maven with implemented features? I want to test it. Thanks

@JamesMcIntosh
Copy link
Author

Hi @Yazon2006,

You can include this dependency using https://jitpack.io and this dependency

com.github.Agrimap:RxPaparazzo:2.x-files-SNAPSHOT

Otherwise you can checkout the project from my branch and run the following command to install it into your local repository

./gradlew clean rx_paparazzo:install

Dependency would then be

com.github.miguelbcr:RxPaparazzo:2.x-agrimap

Kind regards
James

@miguelbcr miguelbcr merged commit f6e015a into miguelbcr:2.x Mar 1, 2017
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.

4 participants