fix(android): don't set EXTRA_MIME_TYPES when using 'Any' media type on Android

NativeScript / nativescript-imagepicker

Imagepicker plugin supporting both single and multiple selection.

Apache License 2.0

104 stars 74 forks source link

fix(android): don't set EXTRA_MIME_TYPES when using 'Any' media type on Android #301

Closed xuhcc closed 5 years ago

xuhcc commented 5 years ago

PR Checklist

[x] The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
[ ] There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
[ ] All existing tests are passing
[ ] Tests for the changes are included

What is the current behavior?

On Android, even when media type is set to ImagePickerMediaType.Any, it is not possible to select any file except image or video, everything else is greyed out.

What is the new behavior?

You can select any file.

Partly reverts changes made in #290 and #291.

tgpetrov commented 5 years ago

Hi @xuhcc Can you suggest other types that you would expect to get picked by image picker that are not image or video?

xuhcc commented 5 years ago

I'm using it for selecting text files (text/plain).

tgpetrov commented 5 years ago

We are trying to have as much similarity between android and iOS as possible and as we are using QBImagePicker on iOS which doesn't support text files having that option only on android would bring a difference between the two. What's more, the plugin's name (imagepicker) does not imply having support for all possible file types and that's why we'd prefer to keep the option "any" only in the images/videos context. Have you tried nativescript-mediafilepicker, it seems to be similar and supports different types of files and not only images on both platform?

xuhcc commented 5 years ago

Yes, I tried every plugin available, and none of them suited my needs, so I decided to add support for all media types to nativescript-imagepicker. I don't remember exactly what was wrong with nativescript-mediafilepicker, but probably it was the UI.

As an alternative approach, we can introduce a new option like allowArbitraryFileSelection which will be available only on Android, and leave current behaviour as it is, limiting user to images and videos by default.

xuhcc commented 5 years ago

I can also move all logic related to EXTRA_MIME_TYPES to a separate method on ImagePicker class to allow easy overriding in subclasses.

tgpetrov commented 5 years ago

Hi @xuhcc As stated earlier, we prefer to keep the plugin with only image picking capabilities especially given that the iOS library does not support other file types. You are free to try re-evaluate other plugins like the above-mentioned nativescript-mediafilepicker or even fork the nativescript-imagepicker plugin and add changes as you see fit. We will gladly welcome any PRs related to adding new image related capabilities and bug fixes.

xuhcc commented 5 years ago

@tgpetrov

I use nativescript-imagepicker plugin in an open-source project, so forking is an option of last resort for me because I'm trying to keep maintenance costs at a minimum. Hopefully we can agree on this change: #304