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

FLImageView notified with FLImageLoadedNotification after image is loaded #1

Open
riazb opened this issue May 31, 2011 · 4 comments
Open
Assignees

Comments

@riazb
Copy link

riazb commented May 31, 2011

FLImageView registers for notifications on FLImageLoadedNotification.

The way things are wired up, after an FLImageView is notified and the image for its URL is fetched etc, it would still get notified on future loads of other images as it only unregisters itself from notifications on dealloc.

Suggestion: Insert removeObserver after an image is received and assigned - hence further notifications won't be dealt with.

@ghost ghost assigned anoopr May 31, 2011
@anoopr
Copy link
Owner

anoopr commented May 31, 2011

I'll have to think about this one. It could potentially raise issues in UITableViewCells where the FLImageViews are reused. If we unregister in imageLoaded, we need to register in loadImageAtURLString. If you are quickly scrolling through, the same FLImageView might register itself twice before an image comes back and then only unregister itself once.

If you have some ideas on how this could be implemented safely, then I'd definitely welcome a pull request.

Thanks for the report!

@riazb
Copy link
Author

riazb commented May 31, 2011

If you call:
[[NSNotificationCenter defaultCenter] removeObserver:self];
before adding an observer, the notification center will clear any registrations matching the FLImageView instance.

In that case, no need to addObserver in the init{} methods. Off the top of my head, I'd structure the loadImageAtURLString method as follows:

  1. Remove observer
  2. Add observer
  3. Use singleton to request image
  4. If we have an image, remove observer
  5. If not, we wait

On the imageLoaded,

  1. Remove observer

I would leave removeObserver on dealloc in case the FLImageView instance is destroyed before it is notified following a recent request.

I'll give it a go.

@avrahamshukron
Copy link

In FullyLoaded, Add user info for each notification with the URL string of the relevant request.
This way, each FLImageView could check if it's relevant to it or not.

@tamoyal
Copy link

tamoyal commented Jan 27, 2012

+1 to avrahamshuk. It seems like this code was designed under the assumption that when one image loaded, all the rest would likely load really quickly. If I'm not mistaken the following is possible - 10 images call to load their data and the activity shows. Now one image comes back and every image gets the notification. So every image now thinks it's not loading. However, if the 9 other images come back pretty quickly (good network connection or cached would easily do this) then you would get all of those 10 notifications around the same time and therefore all the images would look as though they were loading just fine. Any reason you chose to design this way? The rest of the component looks very good. Thanks!

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

4 participants