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

Limit number of files that can be selected #60

Merged
merged 10 commits into from
Jul 18, 2024

Conversation

ghost
Copy link

@ghost ghost commented Jul 10, 2024

Mentioned in this issue: #50.

I tried to tackle this, but there are multiple issues:

Doesn't work:

  • JVM
  • JS / Wasm
  • macOS

Partially works:

  • Android (Only works for images and media, not for regular files)
  • iOS (Only works for images and media, not for regular files)

These results are not great. I think we can implement the limit ourselves by keeping track of the amount of selected files and then either not letting the user select more or deselecting the oldest selection, FIFO style.

A possible implementation for this on JVM (Windows) for example is to have a coroutine running the whole time the picker is open and querying fc.selectedFiles and when it goes over the limit apply the limit strategy to keep the selected files at or below the max.

I haven't looked into whether this manual handling is possible for all platforms. I could take a look, but I'm not sure if maintaining this for all platforms is something that you want to do @vinceglb.

There might also be better options, if anyone knows any add a comment to this PR and I will check it out.

@LaatonWalaBhoot
Copy link

Please take a look at https://github.com/onseok/peekaboo
Maybe helpful

@vinceglb
Copy link
Owner

I updated your PR with these changes:

  • maxItems property is now nullable and must be contained between 2 and 50.
  • I modify the default value from Int.MAX_VALUE because it crashes on Android and put it to null.

On Android, the value must not exceed MediaStore.getPickImagesMaxLimit() but this method is only accessible from Android API 33... So I put 50 as the limit, but it's an arbitrary number. And 2 because putting 1 doesn't make sense to me when using PickerMode.Multiple.

I tested on Android and iOS, with and without maxItems set, and it works fine for both versions 👍

@santiwanti, what do you think of the changes?

@vinceglb vinceglb marked this pull request as ready for review July 13, 2024 00:25
@vinceglb vinceglb linked an issue Jul 13, 2024 that may be closed by this pull request
@ghost
Copy link
Author

ghost commented Jul 15, 2024

@vinceglb They look good. The PR was more a prototype to see on which platforms could handle maxItems nicely, but I hadn't tested it at all.

  • Making maxItems be greater than 2 seems reasonable. I had thought we could handle it for the user, but it's probably easier if they do the check anyway to call the single or multiple.
  • I will look into how we can handle a limit nicely without an arbitrary value of 50. There might be something that can be used in older API versions, but this can be done in another PR if you want to merge this soon.

@LaatonWalaBhoot
Copy link

Just to add most apps keep a limit of 10 to 30 for a single selection. Perhaps to ensure memory management. 50 is a very good number to have though. Might be a issue for HEIC image files.

@vinceglb
Copy link
Owner

vinceglb commented Jul 17, 2024

@santiwanti

  • I've thought about putting maxItems range from 1 like in your original PR and I think it's a good idea! It's probably easier for the developer to handle this case. I've updated the code in that way
  • If you have an idea on how to handle the max items limit, I'd prefer to wait for your solution before merging we'll create another PR to manage max items limit nicely

@LaatonWalaBhoot
Thanks for your feedback! The picker returns PlatformFiles with nsURL / Uri pointing at each file. The library doesn't load the bytes from the files into memory. So, I think that even 50 files will not cause any problem. The library loads bytes into memory only if we call readBytes() on the platformFile.

@vinceglb
Copy link
Owner

Finally, I'm going to merge this PR today to create version 0.7.0 and we'll create another PR to manage max items limit nicely later.

…max_files

# Conflicts:
#	filekit-core/src/jvmMain/kotlin/io/github/vinceglb/filekit/core/platform/mac/MacOSFilePicker.kt
@vinceglb vinceglb merged commit 585361b into vinceglb:main Jul 18, 2024
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.

Add Feature to Limit number of Images that can be picked
2 participants