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

Add web Images support (proper way) #33

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ogonzalezadell
Copy link
Contributor

Add support for web images with disclosure indicator

This pull request is a bit more complex :

  • Demo project includes JPSThumbnail as a Pod (also SDWebImage as dependency)
  • SDWebImage and disclosure indicator has been added
  • Demo project has been updated to recommended settings
  • Demo project has been updated to display the new features

Notes:

  • All changes have been separated in different commits to make it easier to understand
  • To run the project first you must run "pod install" command and open the xcworkspace file
  • If this Pull request is accepted I would recommend updating the pod version and the readme

Update Demo project to recommended settings
Added JPSThumbnailAnnotation dependencies as a local pod file
Update XcodeProject with Pods (after pod install)
Updated PodSpec to include SWebImage as a pod dependency
Added imageUrl parameter to thumbnail and SDWebImage (async web image
loading from url) logic with activity indicator as recommended in their
github repository.
Added a new example of an image loaded from a url.
Also added the example for content mode (aspect fit)
@jpsim
Copy link
Owner

jpsim commented Jan 17, 2017

This library has so far been dependency-free, which has greatly minimized the maintenance effort, which is nice given that I haven't personally used this code in several years.

Instead of bundling SDWebImage with this library, are there instead changes that could be made to JPSThumbnailAnnotation that would allow it to better interoperate with whichever networking or image caching library a user prefers and likely already has configured in their project?

I generally prefer making APIs more modular and composable.

@ogonzalezadell
Copy link
Contributor Author

Okey, I got your point. Have you got any recommendation? If not I will try to work in a more creative way and see if i can make it different.

@jpsim
Copy link
Owner

jpsim commented Jan 17, 2017

Given that you've had more exposure to this library than I have these last few years, I'm hoping you'd have some experience to share. What's preventing this library from working well with an outside image source, like SDWebImage? Does everything work well if you just asynchronously set the image?

@ogonzalezadell
Copy link
Contributor Author

It seems that is quite difficult to access the ImageView to load images, it should be exposed somehow to allow users to change it manually when the thumbnails are loaded in the map. As I'm not a very experienced developer I do now want to make big "tricky" functions, but perhaps could be a nice idea to implement something similar to tableview delegate methods.

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.

2 participants