Package Review

[Johan-dutoit]

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

• Rip out all permissions and shift the responsibility to the user (we'll add best practices
• Modernise and clean up the code base (enforcing some standards too)
• Getting additional maintainers onboard

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

[angimenez]

I like your plan 👍

[HansBouwmeester]

Leaving it to the user to sort out permissions is the way to go! There's lots of packages providing assistance, e.g. react-native-permissions.

Would be a real shame seeing this package be dropped just because the permissions jungle is getting in the way!

[tsiory]

I agreed with you.

[macavity23]

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.

[Johan-dutoit]

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:

• Swift for the iOS Package
  • This should use the new PHPicker
• Kotlin for the Android Package
• TypeScript for the JavaScript package

That being said, I don't have the time to fully commit to this now. If you are keen to assist, please reach out.

[ravirajn22]

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

• Let permission be within the library for iOS, for android we have permission android built in to react-native. We should not force lib users to add another third party library for the sake of permission.
• Lets rewrite file by file from objective C to swift. Swift is far-far better language to grasp than objective c.
• Whereas in case of Android lets stick with Java because there is not much advantage you gain on moving to Kotlin from what I know just few syntactic sugars.

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?

[Johan-dutoit]

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

[douglasjunior]

About expo-image-picker, I don't think that is a good idea for bare React Native projects, because it require react-native-unimodules.

[ravirajn22]

@Johan-dutoit I suggest we remove ImagePicker.showImagePicker, people if they want can show menu of their choice like using ActionSheetIOS from react-native or simple buttons etc. It's a burden for this library. Community your thoughts?

[Johan-dutoit]

@ravirajn22 Totally agree.

Especially with trying to support all the theming of the action sheet.

Having the following two functions would suffice:

• launchCamera, and
• launchImageLibrary

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.

[tastafur]

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

[Johan-dutoit]

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.

[ravirajn22]

Also list here any master branch feature unavailable on vnext branch which is a deal breaker to upgrade. We will brainstorm and consider to add to vnext.

[ravirajn22]

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

[ravirajn22]

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

[AronBe]

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 react-native-document-picker

[ravirajn22]

@AronBe I don't get your feature request. What customButton?

[AronBe]

@ravirajn22 it is in the response object of ImagePicker, but based on vnext documentation, it is missing

ImagePicker.showImagePicker(options, (response) => {
  console.log('Response = ', response);

  if (response.didCancel) {
    console.log('User cancelled image picker');
  } else if (response.error) {
    console.log('ImagePicker Error: ', response.error);
  } else if (**response.customButton**) {
    console.log('User tapped custom button: ', response.customButton);
  } else {
    const source = { uri: response.uri };

    // You can also display the image using data:
    // const source = { uri: 'data:image/jpeg;base64,' + response.data };

    this.setState({
      avatarSource: source,
    });
  }
});

[Johan-dutoit]

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

[Johan-dutoit]

Closing this, as tons of progress has been made regarding this issue. Many thanks to everyone involved, especially @ravirajn22.