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

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

Open
3 of 15 tasks
Vounts opened this issue Dec 27, 2023 · 16 comments
Open
3 of 15 tasks

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

Vounts opened this issue Dec 27, 2023 · 16 comments
Labels

Comments

@Vounts
Copy link

Vounts commented Dec 27, 2023

Plugin(s)

  • Android Foreground Service
  • Android Battery Optimization
  • App Update
  • Background Task
  • Badge
  • Cloudinary
  • Datetime Picker
  • File Opener
  • 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

@robingenz
Copy link
Member

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 robingenz changed the title feat: /Filepicker - Able to filter out .webp and .gif on .pickImage method feat(file-picker): pass mime types to pickImages(...) Dec 27, 2023
@stephan-fischer
Copy link
Contributor

@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
Copy link
Member

@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
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

Wait, shouldn't it only be necessary to define excluded types? The included types are already defined by the plugin methods, for example:

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

stephan-fischer commented Jan 18, 2024

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
Copy link
Member

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
Copy link
Contributor

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

@robingenz
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants