NYTPhotoViewer icon indicating copy to clipboard operation
NYTPhotoViewer copied to clipboard

Proposal: Interactive reload and loading indicator

Open kimar opened this issue 9 years ago • 7 comments

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:

  1. Right now it's not feasible to interact with photos while downloading a hi-res version AND replace the hi-res version after download while still maintaining zoom-scale and position so that the user interaction isn't interrupted when replacing the lo-res photo with the hi-res one.
  2. Display a progress to the user to inform him about when a file has been downloaded / will be replaced with a hi-res version.
  3. Enable configurable shadow on UINavigationBar, the same way the shadow (gradient) is composed on the lower tool bar. This will result in a unified user experience, also the user will now be able to see the buttons and loading indicator on the upper part of the screen, even if a bright photo is being displayed.

Sneak peek: The solution

out-1

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:

- (CGFloat)photosViewController:(NYTPhotosViewController *)photosViewController
       maximumZoomScaleForPhoto:(id<NYTPhoto>)photo

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's navigationBarGradient property, a shadow can be added to solve this problem.

kimar avatar Feb 07 '16 23:02 kimar

We might want to also expose the progress/loader using a proper datasource https://github.com/NYTimes/NYTPhotoViewer/issues/163

kimar avatar Mar 01 '16 01:03 kimar

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 avatar Mar 04 '16 14:03 baswenneker

@baswenneker NYTPhotoViewer isn't downloading the image, this is done in our framework which then leverages NYTPhotoViewer again. What we're doing is basically an NSURLSession request for the image, in case it's not cached and will be downloaded we're propagating the progress to the the indicator in NYTPhotoViewer. We're using a custom subclass of NYTPhotoViewer and to reset the individual progress for every single using NYTPhotoViewerDelegate e.g.

- (void)photosViewController:(NYTPhotosViewController *)photosViewController
          didNavigateToPhoto:(FVImage <NYTPhoto> *)photo
                     atIndex:(NSUInteger)photoIndex {
    self.overlayView.loadingIndicatorView.progress = 0.0f;
}

The progress property is then update in our download operation.

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.

kimar avatar Mar 07 '16 01:03 kimar

I found an issue with this fix @kimar.

I am using lazy loading and calling updateImageForPhoto: to show the initial image if it's not already loaded.

When I do this it does not set the initial imageView.size here:

- (void)updateImage:(UIImage *)image imageData:(NSData *)imageData {
...
#ifdef INTERACTIVE_RELOAD
    self.imageView.frame = CGRectMake(0, 0, imageTouse.size.width, imageTouse.size.height);
#endif
...
}

I updated your code to handle this case as below:

- (void)updateImage:(UIImage *)image imageData:(NSData *)imageData {
...
#ifdef INTERACTIVE_RELOAD 
    self.imageView.frame = CGRectMake(0, 0, imageToUse.size.width, imageToUse.size.height);
#else
    if (CGSizeEqualToSize(self.imageView.frame, CGSizeZero)) {
        self.imageView.frame = CGRectMake(0, 0, imageToUse.size.width, imageToUse.size.height);
    }
#endif
...
}

Also this may further be an issue as described in #206.

egarlock avatar Aug 17 '16 20:08 egarlock

Hey Marcus (@kimar),

Are you using a constant for your - (CGFloat)photosViewController:(NYTPhotosViewController *)photosViewController maximumZoomScaleForPhoto:(id<NYTPhoto>)photo ?

I was experimenting with using a dynamic max scale based on size of image but just realized you may be using a constant here.

egarlock avatar Aug 29 '16 20:08 egarlock

Great work @kimar, any reason why this kept open until now? looks like a great addition.

awartani avatar Aug 07 '19 20:08 awartani

@awartani it's been years since I've been in touch with NYTPhotoViewer and this PR in particular. I'm not sure about the state and if it even aligns with the roadmap of NYTPhotoViewer, also IMO it still has quite an RFC character and probably requires significant work until it's mergeable.

Unfortunately I can't provide anymore help on this currently.

kimar avatar Aug 09 '19 06:08 kimar