capacitor-plugins icon indicating copy to clipboard operation
capacitor-plugins copied to clipboard

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

Open Vounts opened this issue 1 year ago • 16 comments

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.

Vounts avatar Dec 27 '23 13:12 Vounts

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?

robingenz avatar Dec 27 '23 15:12 robingenz

@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 ;) ?

stephan-fischer avatar Dec 29 '23 15:12 stephan-fischer

@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.

robingenz avatar Dec 29 '23 21:12 robingenz

@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.

stephan-fischer avatar Dec 30 '23 19:12 stephan-fischer

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.

robingenz avatar Jan 01 '24 12:01 robingenz

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?

stephan-fischer avatar Jan 16 '24 20:01 stephan-fischer

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.

robingenz avatar Jan 16 '24 20:01 robingenz

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 😊

stephan-fischer avatar Jan 17 '24 08:01 stephan-fischer

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 avatar Jan 17 '24 15:01 robingenz

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[];

robingenz avatar Jan 17 '24 15:01 robingenz

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?

stephan-fischer avatar Jan 17 '24 17:01 stephan-fischer

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.

robingenz avatar Jan 18 '24 08:01 robingenz

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?

stephan-fischer avatar Jan 18 '24 09:01 stephan-fischer

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.

robingenz avatar Jan 18 '24 12:01 robingenz

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

stephan-fischer avatar Jan 18 '24 14:01 stephan-fischer

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

robingenz avatar Jan 18 '24 15:01 robingenz