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

fixed the bug that Gallery picker option is not showing #53

Merged
merged 6 commits into from
Feb 11, 2021

Conversation

datdescartes
Copy link
Contributor

@datdescartes datdescartes commented Feb 10, 2021

close #20 #52

Bug

Cause:

Wrong Intent for gallery

Reproduce

please refer #20

val intent = CropImage.activity()
    .setCropShape(CropImageView.CropShape.RECTANGLE)
    .setAspectRatio(1, 1)
    .setFixAspectRatio(true)
    .setGuidelines(CropImageView.Guidelines.ON)
    .setCropMenuCropButtonTitle("OK")
    .getIntent(this)
startActivityForResult(intent, 123)

How the bug was solved:

Copy the code from legacy lib

@Canato Canato self-requested a review February 10, 2021 09:01
kaustubhkp
kaustubhkp previously approved these changes Feb 10, 2021
Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! really happy to see more people putting hands on to improve the library

I just worry cause this code was changed in #5 to fix another issue. And we need to be sure we didn't bought this problem back.

Could you point me of the tests/devices/OSs you did?

@ravindu1024 @akkie2016 could take a look and check if the issue returned on this version?

cropper/src/main/java/com/canhub/cropper/CropImage.java Outdated Show resolved Hide resolved
cropper/src/main/java/com/canhub/cropper/CropImage.java Outdated Show resolved Hide resolved
@Canato Canato self-assigned this Feb 10, 2021
@Canato Canato added the bug label Feb 10, 2021
@datdescartes
Copy link
Contributor Author

datdescartes commented Feb 10, 2021

@Canato I have found that Android 11 requires permission to get package info.

I've updated the PR that combines the fixes in #5 and the legacy code with permission fixed.
This maybe also fix the bug that camera not show in some devices. (#52 )

The huawei line is removed as getGalleryIntents() always return notEmpty-list.

Tested in Android 11(pixel 4) and Android 7(Xperia)

@Canato
Copy link
Member

Canato commented Feb 10, 2021

Nice work!

Some bits, please update the CHANGELOG with

### Fixed
- [Galley option not showing](https://github.com/CanHub/Android-Image-Cropper/issues/20)
- [Camera option not showing](https://github.com/CanHub/Android-Image-Cropper/issues/52)

And remove the #20 comments, uncommenting the related code.

  • CropImage.java line 156 // TODO Issue #20
  • fragment_main.xml line 48 <!-- Todo Issue #20 --> and uncomment the code
  • CropImageViewFragment line 75 // TODO Issue #20 and uncomment the code.

My Test

Running the sample app, after uncomment the above, if I click the image search I cannot see the galley option
photo5846150917873841738

Pixel 3, Android 11

Are you hitting something similar? or is working for you? Could provide some video/screen if is working? Maybe is my device issue and I can check

@Canato
Copy link
Member

Canato commented Feb 10, 2021

@datdescartes

@datdescartes
Copy link
Contributor Author

@kaustubhkp Sorry, I don't have pixel 4 in my hand now. Will try it on weekend.
It seems the android Q's bug that only shows up maximum 2 items.

I have added the workaround, could you test if it works.

Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! now is working like a charm for me!

Great add to the library @datdescartes

To merge we just need to polish some bits

1. Please update the CHANGELOG with

### Fixed
- [Galley option not showing](https://github.com/CanHub/Android-Image-Cropper/issues/20)
- [Camera option not showing](https://github.com/CanHub/Android-Image-Cropper/issues/52)

2. Remove the #20 comments, uncommenting the related code.

  • CropImage.java line 156 // TODO Issue #20
  • fragment_main.xml line 48 <!-- Todo Issue #20 --> and uncomment the code
  • CropImageViewFragment line 75 // TODO Issue #20 and uncomment the code.

3. Use the CommonVersionCheck class

  • change Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q to CommonVersionCheck.INSTANCE.isAtLeastQ29()

Question:

On my test (using the sample app) after choosing a image from camera/gallery the image do not load into the screen.
I would say this is a sample bug that I need to fix later, not related to your changes and out of the scope of this PR.
But can you confirm me that you was able to load a image from camera/gallery on your tests?

cropper/src/main/java/com/canhub/cropper/CropImage.java Outdated Show resolved Hide resolved
@datdescartes
Copy link
Contributor Author

Hi @Canato
I've just done the 1,2,3 above.
I have also fixed the sample bug. Confirmed that the image from picker show on screen. (Using fixed sample app)

@kaustubhkp
Copy link

@datdescartes Awesome Job...

I did some testing on Google Pixel 4 XL device and it works like charm for me

Now It shows camera, Gallery and Files option for me...I verified after cropping image it sending data inside onActivityResult as well :)

screenshot

Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid amazing work!

If you are feeling so, join us as team member =)
https://github.com/CanHub/Android-Image-Cropper/discussions/42

Next release coming!

@Canato Canato merged commit 054cf60 into CanHub:main Feb 11, 2021
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.

Gallery option is not showing
3 participants