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

Also set the webURL to nil when resource is nil #598

Merged
merged 1 commit into from
Feb 21, 2017

Conversation

pNre
Copy link
Contributor

@pNre pNre commented Feb 20, 2017

Hi,
while using Kingfisher in a collection view I noticed that in some cases the image views weren't updated as expected.

This is what I'm doing:

  • Dequeue a reusable cell containing an image view
  • Set the image to a certain image-url using KF
  • At some point the cell gets reused and the download KF download canceled
  • Set the image to nil with a placeholder

What I expect at this point is for the image view to display the placeholder, what happens is that it still displays the image at image-url.

I looked a bit into the setImage method in ImageView+Kingfisher and noticed that the cause of this issue might be the fact that webURL is not updated when setImage is called with a nil resource thus letting the completion block of the download task update the image even if the operation was cancelled and the setImage called again.

Also, the same this is probably happening for UIButton and NSButton extensions but I wanted to open this PR before editing any other method.

Delete the associated WebURL when a nil `Resource` is set on an ImageView.
This should prevent that the imageView is updated with the wrong image if a download previously started completes after the image is set again to nil.
@codecov-io
Copy link

codecov-io commented Feb 20, 2017

Codecov Report

Merging #598 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #598      +/-   ##
==========================================
+ Coverage   74.71%   74.72%   +0.01%     
==========================================
  Files          21       21              
  Lines        2246     2247       +1     
==========================================
+ Hits         1678     1679       +1     
  Misses        568      568

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1add8d8...55a9ca4. Read the comment docs.

@onevcat
Copy link
Owner

onevcat commented Feb 21, 2017

@pNre Good catch. I believe you are right on this.

Could you please also send p-r for button types too? Thank you!

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