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

#59 Fixed the image flashing in the IssueDetailViewController avatars #60

Merged
merged 2 commits into from
Feb 11, 2017

Conversation

chrisbrandow
Copy link
Collaborator

@chrisbrandow chrisbrandow commented Feb 10, 2017

#59

This solution is memory only, and is local to this IssueDetailViewController, but could be moved to a singleton/app delegate.
Or, conversely, we could use an existing cache solution. However, this seems like a small thing to pull in another dependency for.

  • [String: UIImage] property to IssueDetailViewController
    • check it when setting cell.avatarImageView.image
    • or set it after downloading remote image
  • added setRemoteImage method with a "preferredImage" argument

@chrisbrandow chrisbrandow changed the title Fixed the image flashing in the IssueDetailViewController avatars #59 Fixed the image flashing in the IssueDetailViewController avatars Feb 10, 2017
@mccarron
Copy link
Collaborator

This solves the issue on IssueDetailViewController but we still have the issue with the avatar flashing on the CallScriptDetailViewController.

So maybe we should instead of AppDelegate or a Singleton just use NSCache and have RemoteImageView handle the caching completely taking that logic out of the view controllers at all?

@subdigital
Copy link
Collaborator

Definitely would prefer using NSCache or similar for this sort of thing because in low memory conditions it will automatically get flushed, whereas we'd have to handle that case here. I'll update this PR w/ some modifications.

*  `[String: UIImage]` property to `IssueDetailViewController`
    * check it when setting `cell.avatarImageView.image`
    * or set it after downloading remote image
* added `setRemoteImage` method with a "preferredImage" argument

This solution is memory only, and is local to this `IssueDetailViewController`, but could be moved to a singleton/app delegate.
or we could use an existing cache solution. Seems like a small thing to pull in another dependency for.
@subdigital subdigital merged commit 20dd53a into master Feb 11, 2017
@subdigital
Copy link
Collaborator

@chrisbrandow thanks for starting this off!

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.

None yet

3 participants