-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Don't let Glide clear the old image on Android ImageView unless we get an error #23313
base: main
Are you sure you want to change the base?
Conversation
Hey there @albyrock87! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
@mattleibow I guess you're the expert on this :) |
this.callback = callback; | ||
} | ||
|
||
@Override | ||
protected void onResourceCleared(@Nullable Drawable placeholder) { | ||
this.view.setImageDrawable(placeholder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placeholder
is always null
here because we never set it on the configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other suggestion, I think they build maui.aar
and commit it? So, developers in main can work without the gradle build succeeding?
src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/glide/MauiCustomViewTarget.java
Show resolved
Hide resolved
src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/glide/MauiCustomViewTarget.java
Outdated
Show resolved
Hide resolved
85bc395
to
185bfe5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks OK to me, but we might need someone to manually test. 👍
Description of Change
This post explains very well how Glide works: they replace the image with a placeholder (
null
if not specified in the configuration) right before loading the new one.This causes a slight flickering on the
ImageView
while changing the image source.To reproduce this, and verify the fix, I've used the code in issue #6625, which apparently is not entirely fixed on Android.
We have to clear the image only if there was an error loading the new one.
To test the error use case I've added a fake url in the code sample above (i.e.
"https://what.is.this.url.com/nonsenseimage1234567.jpg"
).I could have cleared the image without creating the
cleared
field, but I wanted to keep part of the existing behavior somehow.Before
before.mov
After
after.mov