-
Notifications
You must be signed in to change notification settings - Fork 380
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
Proposal: Interactive reload and loading indicator #160
base: develop
Are you sure you want to change the base?
Conversation
# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…r-Shadow Conflicts: Pod/Classes/ios/NYTPhotosOverlayView.m
…Shadow Conflicts: Pod/Classes/ios/NYTPhotosOverlayView.h Pod/Classes/ios/NYTPhotosOverlayView.m
…ar-Shadow # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
We might want to also expose the progress/loader using a proper datasource #163 |
Hi @kimar, this is exactly what I needed! I checked out your code but I can't find how and where to trigger loading the high resolution image. Can you give an example of how you use it in your example? Thanks in advance! |
@baswenneker
The Also bear in mind that replacing the image after downloading the hi-res version currently requires the final image to have the same aspect ratio, but for our use case this hasn't been an issue so far. |
167eabd
to
b64916f
Compare
I found an issue with this fix @kimar. I am using lazy loading and calling When I do this it does not set the initial imageView.size here:
I updated your code to handle this case as below:
Also this may further be an issue as described in #206. |
Hey Marcus (@kimar), Are you using a constant for your I was experimenting with using a dynamic max scale based on size of image but just realized you may be using a constant here. |
UIImageView *imageView = scalingImageView.imageView; | ||
|
||
CGFloat imageHeight = CGRectGetWidth(imageView.frame) * CGRectGetHeight(imageView.frame) / CGRectGetWidth(imageView.frame); | ||
CGFloat imageWidth = CGRectGetHeight(imageView.frame) * CGRectGetWidth(imageView.frame) / CGRectGetHeight(imageView.frame); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the goal, in this calculation, of performing GetHeight * GetWidth / GetHeight
? Isn't that equivalent to just GetWidth
?
Great work @kimar, any reason why this kept open until now? looks like a great addition. |
@awartani it's been years since I've been in touch with NYTPhotoViewer and this PR in particular. Unfortunately I can't provide anymore help on this currently. |
Preamble
I've solved three problems we've been facing when using NYTPhotoViewer, initially I planned to file three PRs but as it turns out all of those problems are connected and it's more convenient to present them all together then separately.
Rationale
We're using NYTPhotoViewer to display photos, when a hi-res version of the photo to be shown is not available, we're presenting NYTPhotoViewer from a reference view, using a thumbnail, and asynchronically download the hi-res version.
This PR solves two problems for us:
Sneak peek: The solution
Problem 1 solved: Interact with lo-res and replace with hi-res without distraction
We want to provide a first class experience when viewing photos, this also requires us to make it possible for users to interact with the lo-res photo (e.g. zooming and scrolling) and preserving the position as well as zoomscale but replacing the photo with a hi-res version when available.
This is only available when providing a custom zoom scale using:
Problem 2 solved: Visualizing download progress
When downloading the hi-res version of a photo, we want to provide a non-blocking, non-interruptive way to propagate this progress to the user. We've decided to go with a 2px indicator at the top of the view controller.
Problem 3 solved: UINavigationBar buttons and progress indicator not visible on bright photo
Because the UINavigationBar doesn't come with a shadow, it's white controls aren't visible on a bright background. By setting
NYTPhotosOverlayView
'snavigationBarGradient
property, a shadow can be added to solve this problem.