feat(file-picker): pass mime types to `pickImages(...)`

Vounts commented 11 months ago

Plugin(s)

[ ] Android Foreground Service
[ ] Android Battery Optimization
[ ] App Update
[ ] Background Task
[ ] Badge
[ ] Cloudinary
[ ] Datetime Picker
[ ] File Opener
[X] File Picker
[ ] Managed Configurations
[ ] NFC
[ ] Photo Editor
[ ] Screen Orientation

Current problem

As a user I can select a .webp and .gif image using the .pickImage method

Preferred solution

put a type parameter in .pickImage

.pickImage({
type: ['image/jpg']
});

to filter out only JPG images

Alternative options

No response

Additional context

No response

Before submitting

[X] I have read and followed the feature request guidelines.
[X] I have attached links to possibly related issues and discussions.

robingenz commented 11 months ago

I think we could implement this for Android but I'm not sure if it's possible on iOS. Just out of interest: Why don't you use the pickFiles(...) method?

stephan-fischer commented 11 months ago

@robingenz on iOS is only PHPickerFilter available.

Apples says:

We don't recommend filtering the picker using explicit file formats. It could be hard for users to understand why some of their photos aren't included in the picker.

However, maybe we could integrate a filter function like forcePhoto: true / forceJPEG: true ?

and in Swift screenshots, and gifs can be excluded:

configuration.filter = .all(of: [
        .images,
        .not(.screenshots), // screenshot === png
        .not(.playbackStyle(.imageAnimated)) // === gif
])

but it means, on iOS its possible to filter for gif:

configuration.filter = .playbackStyle(.imageAnimated) means maybe the function forceGIF: true

And the filter function can filter screenshots, videos etc - maybe its time for an large iOS filter function ;) ?

robingenz commented 11 months ago

@stephan-fischer Thanks for sharing! I definitely plan to support the filter on iOS. But I don't like options like forcePhoto or forceJPEG. Especially if they are only for one platform. I'll see if I can come up with something better.

stephan-fischer commented 11 months ago

@robingenz I'm looking forward to it! Do you think there is a generic solution to create such complex filtering? Right now I actually need a filter for my app that allows real photos only, and not screenshots, no GIFs - its also good for photographer upload tools etc.

robingenz commented 11 months ago

Perhaps we could map each PHPickerFilter to a mime type. For example, if image/png is passed as the type, then .screenshots is taken as the filter. However, the question is whether it is possible to assign each PHPickerFilter to an individual mime type. If this is not possible, I would suggest a new filter option to which you can pass the possible PHPickerFilters as an array. However, this could quickly become complex, as there is also the not filter.

stephan-fischer commented 10 months ago

Should the user add multiple image types, like png and gif -> .screenshot + .animated? @robingenz Are you planning to develop this feature or are pull requests welcome?

robingenz commented 10 months ago

Should the user add multiple image types, like png and gif -> .screenshot + .animated?

Yes, this would be great, just like the "types" option in pickFiles.

PRs are welcome.

stephan-fischer commented 10 months ago

OK, great! I thought about it a little more. Maybe the best solution would be to have the types array only for android, and a filter array for ios only.

Means

.pickImage({
    types: ['image/png'], // android only
    filter: ['images', 'not:screenshot', 'not:animated'] // ios only - short alternative to have include, exlcude array
});

to have the maximum flexibility? And should we have string values for filter, or better an enum for it like:

filter: [FilePickerFilter.Images, FilePickerFilter.NoScreenshot, FilePickerFilter.NoAnimation]

This filter works also on pickMedia and pickVideo. For filter following exists:

static let bursts: PHPickerFilter A filter that represents assets with multiple high-speed photos. static let cinematicVideos: PHPickerFilter A filter that represents videos with a shallow depth of field and focus transitions. static let depthEffectPhotos: PHPickerFilter A filter that represents photos with depth information. static let images: PHPickerFilter A filter that represents images, and includes Live Photos. static let livePhotos: PHPickerFilter A filter that represents Live Photos. static let panoramas: PHPickerFilter A filter that represents panorama photos. static let screenRecordings: PHPickerFilter A filter that represents screen recordings. static let screenshots: PHPickerFilter A filter that represents screenshots. static let slomoVideos: PHPickerFilter A filter that represents slow-motion videos. static let timelapseVideos: PHPickerFilter A filter that represents time-lapse videos. static let videos: PHPickerFilter A filter that represents video assets.

If separate options are better, than we need another issue only for ios and filter 😊

robingenz commented 10 months ago

Interesting idea. In principle, I would have no problem with it. What I don't like is that every enum would have to exist twice, i.e. Screenshot and NoScreenshot. 🤔

I would prefer the following:

/**
 * The list of filters to apply to the picker.
 * 
 * Only available on iOS (14+).
 * 
 * @since 5.4.0
 * @see https://developer.apple.com/documentation/photokit/phpickerfilter/3952798-all
 */
includeFilters?: Filter[];
/**
 * The list of filters to exclude from the picker.
 * 
 * Only available on iOS (14+).
 * 
 * @since 5.4.0
 * @see https://developer.apple.com/documentation/photokit/phpickerfilter/3952802-not
 */
excludeFilters?: Filter[];

robingenz commented 10 months ago

Wait, shouldn't it only be necessary to define excluded types? The included types are already defined by the plugin methods, for example: https://github.com/capawesome-team/capacitor-plugins/blob/06f5f240a724d922c13325a8f67e87405d33730c/packages/file-picker/ios/Plugin/FilePicker.swift#L52

The following should therefore be enough:

/**
 * The list of filters to exclude from the picker.
 * 
 * Only available on iOS (14+).
 * 
 * @since 5.4.0
 * @see https://developer.apple.com/documentation/photokit/phpickerfilter/3952802-not
 */
excludeFilters?: Filter[];

stephan-fischer commented 10 months ago

Yes you are right, but pickMedia can have the includeFilters function, and i think pickImages should have the includeFilters also - to override the image filter. For example if you want only animated images - that is the filter:

 configuration.filter = .playbackStyle(.imageAnimated)

Or this option has only pickMedia?

robingenz commented 10 months ago

You are right. But we should only add the includeFilters option to the pickMedia(...) method. Otherwise, you could also use the pickImages(...) method to select videos, for example.

stephan-fischer commented 10 months ago

Sounds like a plan 😉 means, we can rewrite PickMediaOptions but we have to add specific definitions for

PickImagesOptions and PickVideosOptions
You prefer string in filter, or enum type?
Would you create a new issue for the ios filter? / or is this issue enough?

robingenz commented 10 months ago

I just created a new branch feat/issue-106 (see #121). Feel free to create a PR based on this branch. I already added all necessary types.

Would you create a new issue for the ios filter? / or is this issue enough?

No, this issue is fine.

stephan-fischer commented 10 months ago

Thank you! And on Android we can provide as pull request the mime type param.

robingenz commented 10 months ago

Yes, but for Android I would like to have a separate PR as soon as #121 is merged.

capawesome-team / capacitor-plugins