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

Use one Glide instance per NewsListRecyclerAdapter #1410

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

Unpublished
Copy link
Collaborator

@Unpublished Unpublished commented Apr 1, 2024

Fixes the issue mentioned in #1405 without each view having its own glide instance

Also fixes leaking context via static reference to FavIconHolder


More infos:

Ref: #1405 (comment)
I think the main culprit is having a static FavIconHandler which leaks context.

Using a per fragment instance approach for Glide will still benefit from Glide's lifecycle awareness, pausing requests of not visible fragments. Also fragments context should be aware of configuration changes like theme AFAIK.

Signed-off-by: Unpublished <unpublished@gmx.net>
Signed-off-by: Unpublished <unpublished@gmx.net>
@Unpublished
Copy link
Collaborator Author

@DoHe Can you double check if this branch fixes the issue for you as well?

@DoHe
Copy link

DoHe commented Apr 1, 2024

@Unpublished yes, I can confirm this fixes the favion issues I was seeing 👍

@David-Development
Copy link
Member

@Unpublished Thank you for implementing this so quickly! Looks like a great solution to me. I'm glad that you were able to remove the static reference.

@David-Development David-Development merged commit 525da96 into master Apr 1, 2024
16 checks passed
@delete-merged-branch delete-merged-branch bot deleted the noStaticFavIconHandler branch April 1, 2024 19:39
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.

3 participants