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

Crop Picker doesn't clear its activit settings? #861

Closed
bartoszwolski opened this issue Oct 25, 2018 · 6 comments
Closed

Crop Picker doesn't clear its activit settings? #861

bartoszwolski opened this issue Oct 25, 2018 · 6 comments

Comments

@bartoszwolski
Copy link
Contributor

bartoszwolski commented Oct 25, 2018

Version

Tell us which versions you are using:

  • react-native-image-crop-picker v0.20.3
  • react-native-cli: 2.0.1
  • react-native: 0.55.4

Platform

  • Android

Expected behaviour

Each time cropPicker is opened it respects given parameters.

Actual behaviour

Assume you have 3 places in code to call cropPicker :

  1.  `ImagePicker.openPicker({
         mediaType: "video",
     }).then(media => {
     }).catch(e => {
     });`
    
  2. ImagePicker.openPicker({ multiple: true, compressVideoPreset: "HighestQuality", }).then(media => { }).catch(e => { });

  3. ImagePicker.openPicker({ width: width, height: height, cropping: true }).then(media => { }).catch(e => { });

Now assume you open pickers in presented sequence and I will explain behavior that is happening instead of what is givien in parameters:

3 -> 1 (now mediaType: "video", is ignored and it allows me only to pick images).
3 -> 2 (now multiple is ignored and it allows me to pick single media, and only image as well).
1 - > 2 -(now I can select only single video).

1 - > 3 - Works well.
2 ->1 - Works well.
2 -> 3 - Works well.

So main issue is if we open picker with cropping, then any other picker open attempt results in failure (according to my standards).

You can also do it like this to reproduce issue:

1 -> 2 -> 1 -> 1 - OK
1 -> 2 -> 1 -> 1 ->3 ->1 - Same issue as 3 ->1.

The annyoing thing is that only way to make cropPicker works as expected with option "2" and "1" after opening option "3" is to restart app which is something I cannot allow in my app.

Steps to reproduce

Just add the code I posted to 3 different buttons naming them "1"/"2"/"3" for easier test and press them according to my 'instrcutions' you will imidiatelly see the issue.

Attachments

Nothing was there, that would be needed to be pasted here.

Love react-native-image-crop-picker? Please consider supporting our collective:
👉 https://opencollective.com/react-native-image-crop-picker/donate

@ivpusic
Copy link
Owner

ivpusic commented Oct 25, 2018

do you have some time to help to fix this?

@bartoszwolski
Copy link
Contributor Author

bartoszwolski commented Oct 25, 2018

yes sure, but there is a wierd thing. After I stopped building with command line ('react native run android') and did build with android studio like this :

  1. Open android studio : android folder.
  2. Run command : react-native bundle --platform android --dev false --entry-file index.js \ --bundle-output android/app/src/main/assets/index.android.bundle \ --assets-dest android/app/src/main/res/
  3. Press play in android studio.

Viola Issue is gone, my guess is that with, command line running android and having enable live reload + debug js remotly it might screw things up a bit?

I will try to do prod build, and see if issue exist there.

@bartoszwolski
Copy link
Contributor Author

Actually I take it back, while some of the issues fixed itself, I still have one issue:
1->2
and it results in having only video displayed with single pick.

So definitely issue exist somewhere. If you would like to find a better way to have conversation then in comments I will welcome it, if not I will keep my eye on this site, while trying to see whats going on.

@bartoszwolski
Copy link
Contributor Author

I guess I can see whats going on finally, but thats my guess for now I will need like 20-30 minutes more to see if it fixes all the issues.

So when you setConfiguration, and in code you have multiple calls, from what it seems you need always to reset previous values, when calling .openPicker

For me at the moment workaround is always passing all values that I "EVER" used in my *.js file, even when i don't need them and pass there default values that are visible in PickerModule.java

@bartoszwolski
Copy link
Contributor Author

bartoszwolski commented Oct 25, 2018

Yea I confirm that was my issue.

So for resolving issue there are 2 options:

  1. Behavior was intentional and you actually should check for all arguments that all used across your multiple calls of cropPicker, and add them to each call but when you don't want to use them pass default values.

  2. Behavior is a bug, and there should be update in function :

private void setConfiguration(final ReadableMap options) {...}

so each short if there is should instead fallback to default value, then to value previously set this is example :

mediaType = options.hasKey("mediaType") ? options.getString("mediaType") : mediaType; - BAD BEHAVIOR

mediaType = options.hasKey("mediaType") ? options.getString("mediaType") : "any"; - GOOD BEHAVIOR

since earlier we can see default declaration of String mediaType :

private String mediaType = "any";

Hope this helps someone who is using multiplecalls in his app,
If there is somewhere a line saying that we should always use default values in case of multiple calls in documentations or somewhere on this plugin site I am sorry for all troubles.

@ivpusic
Copy link
Owner

ivpusic commented Oct 25, 2018

fixed here #862

@ivpusic ivpusic closed this as completed Oct 25, 2018
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

No branches or pull requests

2 participants