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

Loading indicator disappears randomly. #447

Closed
hifall opened this issue Sep 20, 2016 · 24 comments
Closed

Loading indicator disappears randomly. #447

hifall opened this issue Sep 20, 2016 · 24 comments

Comments

@hifall
Copy link

hifall commented Sep 20, 2016

I found the loading indicator can disappear sometimes in such a scenario, but I have not found a consistent repro yet.

This happens in a table view with some chat messages.
1 fill the table view with about a dozen messages;
2 inert an image message whose image is managed by Kingfisher by calling:
imageView?.kf_showIndicatorWhenLoading = true
imageView?.kf_indicatorType = .Activity
imageView?.kf_setImageWithURL(...) with a placeholder image;
now the loading indicator shows up;
3 while the image is being loaded, scroll it out of the view port of the table, so that the image is not visible;
4 scroll the image back into the view port to make it visible; now the loading indicator could be missing even when the image is still being loaded.

@onevcat
Copy link
Owner

onevcat commented Sep 20, 2016

Will take a look at it soon.

@onevcat
Copy link
Owner

onevcat commented Sep 21, 2016

@hifall Hi,

After some investigation, I believe there is no issue in Kingfisher's loading indicator behaviors.

The issue you encountered might be related to the changes on collection view/table view in iOS 10. If you are calling Kingfisher's cancelDownloadTask in didEndDisplaying delegate method and resetting the image in collectionView(:cellForItemAt:), you may suffer from this. Since in iOS 10, Apple is using a more aggressive cache way on cells. Now they will not be in reuse queue at the time it goes out of screen, which means the resetting method in collectionView(:cellForItemAt:) may not be called. So the indicator will not show again.

You could try to put your set image code to collectionView(:willDisplay:forItemAt:) instead, to make sure the image is being reloaded and set correctly for that cell.

@onevcat
Copy link
Owner

onevcat commented Sep 21, 2016

You could find more about this change in iOS in session 219

@onevcat
Copy link
Owner

onevcat commented Sep 21, 2016

Some sample code added in #448 to show how to get performance benefit from iOS 10's prefetching cell and new cell life cycle.

@hifall
Copy link
Author

hifall commented Sep 21, 2016

Okay -- thanks for the heads-up. But the issue happened in iOS 9.3.5, instead of 10. Not sure enough at the moment what's going on, yet.

I will take a deeper look into this later.

@hifall
Copy link
Author

hifall commented Sep 23, 2016

So, I did try a few more times on iOS 9.3.5., and I saw the issue again. Although I did not dig through the source code to find the problem.

@pmusolino
Copy link
Contributor

I have the same problem, before Xcode 8 and the transition to swift 2.3, the activity indicator worked properly on all my collectionViews. Now it doesn't work.

@onevcat
Copy link
Owner

onevcat commented Sep 27, 2016

@codeido I can not reproduce this issue on my environment (I tried both iOS 9 and 10).

Can you build a sample project for this? So I can dig into it deeper.

@pmusolino
Copy link
Contributor

@onevcat you used swift 2.3?

@onevcat
Copy link
Owner

onevcat commented Sep 28, 2016

Yes, I tried swift2.3 branch and cannot reproduce it.

But I did find a bug which might stop you from setting the kf_showIndicatorWhenLoading to false, but it should not relate to this issue, if you are only setting it to true.

@pmusolino
Copy link
Contributor

I fixed it on swift 2.3 and iOS 10 calling kf_showIndicatorWhenLoading=true before the method kf_setImageWithURL 👍 :|

@onevcat
Copy link
Owner

onevcat commented Sep 28, 2016

@codeido So may I know what's your original code? Maybe it could be improved.

@pmusolino
Copy link
Contributor

Yeah @onevcat !

My original code was:

cell.imageProduct.kf_setImageWithURL(NSURL(string: variantSelected!.gallery![indexPath.item] ?? ""),
                                             placeholderImage: nil,
                                             optionsInfo: [.Transition(ImageTransition.Fade(0.5)), .ForceTransition])
cell.imageProduct.kf_showIndicatorWhenLoading = true

now is:

cell.imageProduct.kf_showIndicatorWhenLoading = true
cell.imageProduct.kf_setImageWithURL(NSURL(string: variantSelected!.gallery![indexPath.item] ?? ""),
                                             placeholderImage: nil,
                                             optionsInfo: [.Transition(ImageTransition.Fade(0.5)), .ForceTransition])

@hifall
Copy link
Author

hifall commented Sep 28, 2016

@codeido,

I don't see much difference between your code and what I posted at top, except that you pass in non-nil optionsInfo. I tried yours, but the problem stays.

@hifall
Copy link
Author

hifall commented Sep 28, 2016

@onevcat,

Regarding your comment: "But I did find a bug which might stop you from setting the kf_showIndicatorWhenLoading to false, but it should not relate to this issue, if you are only setting it to true.", this is what it's like in our scenario: we have 2 types of images managed using Kingfisher, one that displays a loading indicator when the image is being loaded, and the other one that displays NO loading indicator. So indeed, kf_showIndicatorWhenLoading can be set to false in our case -- could that cause the images that are supposed to display a loading indicator to actually fail to do so?

@hifall
Copy link
Author

hifall commented Sep 28, 2016

@onevcat,

This issue happens around 50% chance on my device. So once you have a fix for the kf_showIndicatorWhenLoading issue you mentioned, maybe I can give it a test here.

@onevcat
Copy link
Owner

onevcat commented Sep 28, 2016

@codeido That's true, you need to set the loading indicator before you begin loading an image.

@hifall I pushed a commit for it in swift2.3 branch. Would you like to give it a try?

@hifall
Copy link
Author

hifall commented Sep 28, 2016

@onevcat,

I am not seeing any new commit from last few hours in swift2.3 branch. Am I missing something here?

@onevcat
Copy link
Owner

onevcat commented Sep 28, 2016

It should be there.

c20e455

I made the change earlier, just didn't push it until now.

@hifall
Copy link
Author

hifall commented Sep 29, 2016

Thanks @onevcat.

I have tested your fix. But the problem still shows.

@onevcat
Copy link
Owner

onevcat commented Sep 30, 2016

@hifall Ummm, checked the code again and not sure about why.

Are you trying to set the indicator and/or image in another thread instead of main thread?

@onevcat
Copy link
Owner

onevcat commented Sep 30, 2016

Or if there is a demo could reproduce it, I'd like to dig it deeper. Currently I cannot reproduce it so it's quite hard to debug.

@onevcat
Copy link
Owner

onevcat commented Oct 13, 2016

@hifall Hi, any progress for this?

@hifall
Copy link
Author

hifall commented Oct 14, 2016

@onevcat,

I have not been able to find time to isolate this issue in a reasonably small project. I will get back to you when I can.

@onevcat onevcat closed this as completed Dec 13, 2016
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

No branches or pull requests

3 participants