Update model on failed request update. #2293
Conversation
| @@ -359,11 +359,12 @@ protected RequestOptions getMutableOptions() { | |||
| Request request = buildRequest(target); | |||
|
|
|||
| Request previous = target.getRequest(); | |||
| if (request.isEquivalentTo(previous)) { | |||
| if (request.isEquivalentTo(previous) | |||
There was a problem hiding this comment.
Add a comment that references #2270 as to why this is necessary.
|
I'm willing to merge this. It's something that might go away in the future if it becomes burdensome, but I can't think of anything that would conflict with this behavior right now at least. We should probably also try to follow up on the flashing when you manually clear() and restart requests separately. Maybe there's something else we can do to mitigate that case as well. |
…led as it can contains data to help the request to succeed.
|
Comment added. To be honest fixing the flashing with clear would be cool as certainly can fix other things, but having devs to always call clear when their model have private fields that can't be exposed due to hashCode() seems way more hackish than this :( Maybe another way would be to have an optional interface on the model to test for equity and call it if the model have it instead of equals for the request restart. Edit: Actually last proposal sounds good, but will require the flashing fix to have everything working as currently :( As even successful requests would be restarted on completed if internal state changes. |
|
Great thanks! Can you file an issue with some details on your views that might help me track down why the flashing is occurring for your use case when you do clear? Is it in a RecyclerView? If so, are the views dimensions variable? An optional interface is definitely worth more thought. Let's try to at least understand the flashing a bit better and go from there. |
Description
Ensure that restarted request use the updated model when failed/cancelled as it can contains data to help the request to succeed.
Motivation and Context
Mostly fixes #2270
Since I had to push a beta for my app to fix some unseen Android O issues and wanted to start to get feedback about Glide 4, I went with this temporary solution to have all working. I know you have not made any decisions yet, but I PR this as is to have a reference since it's now distributed to end users.