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

Get FragmentActivity from ContextWrapper #142

Closed
wants to merge 1 commit into from

Conversation

WMariusz
Copy link

@WMariusz WMariusz commented Jun 7, 2021

close #136

Bug

Cause:

android.view.ContextThemeWrapper cannot be cast to androidx.fragment.app.FragmentActivity

Reproduce

GIVEN: Version 3.1.1 of the library, view have attibute android:theme
WHEN: using CropImageView.setImageUriAsync() method
THEN: method try to cast ContextThemeWrapper to FragmentActivity

How the bug was solved:

get baseContext from context as long as it not FragmentActivity and as long as it is ContextWrapper

@WMariusz WMariusz requested a review from a team as a code owner June 7, 2021 21:04
@WMariusz WMariusz force-pushed the fix/unpack-context-wrapper branch from 5ab9849 to f381f10 Compare June 7, 2021 21:10
@Canato Canato requested a review from nshevchenko June 8, 2021 08:52
@Canato
Copy link
Member

Canato commented Jun 8, 2021

Hey @WMariusz thanks for the PR. I was looking the code, how about we keep it as Context only?

We could change BitmapCroppingWorkerJob() to accept Context instead of FragmentActivity and it should work. So we avoid changing it type.

For this to work we would need to make BitmapCroppingWorkerJob a extension of CoroutineScope this is a little trick, if you are not sure how this is done I could try to find a time this week and do. what you think?

Obs: The CI broke, please check the contribution guide so you can use the Ktlint formatter and update the changelog

@WMariusz WMariusz force-pushed the fix/unpack-context-wrapper branch from f381f10 to def0721 Compare June 8, 2021 20:29
@WMariusz
Copy link
Author

WMariusz commented Jun 8, 2021

Take a look now, CI should pass this time.

I thought about Context + CoroutineScope solution. However i dont know right now how to get CoroutineContext inside view.

You can take a look on my branch, https://github.com/WMariusz/Android-Image-Cropper/tree/feature/migrate-coroutine-scope.

Unpack solution was the fastest way to achive this with minimum impact on codebase.

@WMariusz WMariusz changed the title get FragmentActivity from ContextWrapper Get FragmentActivity from ContextWrapper Jun 8, 2021
@Canato
Copy link
Member

Canato commented Jun 9, 2021

@WMariusz I was planning to add here the suggestions, but was easier to open a new PR based on yours ^^

Can you please check/review if work for you? #143

@Canato
Copy link
Member

Canato commented Jun 10, 2021

Done in #143

@Canato Canato closed this Jun 10, 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.

[BUG] - ContextThemeWrapper cannot be cast to FragmentActivity while setImageUriAsync
2 participants