-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Package Review #1358
Comments
![:octocat: :octocat:](https://github.githubassets.com/images/icons/emoji/octocat.png)
I like your plan 👍 |
Leaving it to the user to sort out permissions is the way to go! There's lots of packages providing assistance, e.g. Would be a real shame seeing this package be dropped just because the permissions jungle is getting in the way! |
I agreed with you. |
Also agree 100%. Permissions is a thing almost every serious app is going to deal with so it might as well be handled properly by react-native-permissions - this is a community project now so I think it's safe to depend on it & link to it. |
I started a fresh package a while ago that will eventually replace this (haven't committed it yet) that has the following opinions (created using react-native-community/bob:
That being said, I don't have the time to fully commit to this now. If you are keen to assist, please reach out. |
@Johan-dutoit appreciate your effort. But trying to completely rewrite the library will take huge effort and has high percentage of failure to complete. I have the following recommendation,
I and many others are ready to contribute if there is guarantee that the pull request will be reviewed, merged to master or rejected in quick time. It should not be left stale. I am keen to assist, how do we move forward? |
@ravirajn22 some good points brought up. I created a new branch for you (and whoever wants to help), called vnext. PR's into this branch will be reviewed and merge if all looks good. |
About |
@Johan-dutoit I suggest we remove |
@ravirajn22 Totally agree. Especially with trying to support all the theming of the action sheet. Having the following two functions would suffice:
As a nice to have, I'll add an example to the example project that uses react-native-action-sheet. Let's do this on the vnext branch. |
Other suggestions to improve @Johan-dutoit , the use of the camera when it is in video mode, the only data we give is the video file but it would also be necessary to know the format, the video quality, resolution and more parameters |
For those interested @ravirajn22 has done a ton of work related to this (which is highly appreciated). These changes live on the vnext branch. Testing and feedback is greatly welcomed! Once sufficient feedback has been received (and testing of course), then we can promote to master and release an alpha on npm (to reach a wider audience). Any issues against vnext should be tagged with the vnext label. |
Also list here any |
@Johan-dutoit there is no comments from users (I think no one has tried vnext branch). What shall we plan next? release a new major beta version using the vnext branch. We should maintain both branches parallel and release versions for sometime and update master docs to both releases. |
@Johan-dutoit can you provide manage issue, write permission (for vnext branch) for me. So that I can push my changes and handle issues promptly. Any major API changes will obviously go through review process. Also we can close this issue. |
Hi, what about adding a customButton to the vnext as that seems currently the way to allow users to have an option to upload an image alongside e.g. a pdf document through |
@AronBe I don't get your feature request. What customButton? |
This comment has been minimized.
This comment has been minimized.
@AronBe please create a new issue with as much detail as possible. It's off topic for this issue and won't easily be able to manage/track it in here. |
Closing this, as tons of progress has been made regarding this issue. |
There's been a fair amount of issues regarding the permissions with this package and it's been painful to keep up with (especially with a mostly unfamiliar codebase to me).
My plan is as follows and I'd like to get opinions before proceeding
It's a big ask now but the package has been stale for too long. the alternative is to completely drop this package in favour of expo-image-picker.
Suggestions welcome
The text was updated successfully, but these errors were encountered: