evgenyneu / Auk

An image slideshow for iOS written in Swift.
MIT License
277 stars 44 forks source link

When I try to show only 1 image, it doesn't get loaded and shown #80

Closed Ariandr closed 5 years ago

Ariandr commented 5 years ago

I load image urls dynamically and show them in my scroll view. So, it can be 1+ images. It works when I show 2 or more images, but when I show only one, it doesn't load and doesn't show the image. The placeholder remains active.

(1) For example, it doesn't work:

myScrollView.auk.show(url: myImageUrlString)

(2) But that works and scrolls between the correct image and an error image, or 2 correct images, or when I add them in for loop:

myScrollView.auk.show(url: myImageUrlString)
myScrollView.auk.show(url: "") // here shows errorImage

I tested through breakpoints and different scenarios. The image URL in (1) is the same as in (2).

Also, I tried printing images.count and it shows 0 images in (1) and 2+ images in (2) (when there are 2+ correct images).

DispatchQueue.main.asyncAfter(deadline: .now() + 10) {
    print("IMAGES COUNT:", self.myScrollView.auk.images.count)
}

Maybe I don't understand something and I need to set some property when I try to display only 1 image in order for it to work correctly? Thank you in advance.

evgenyneu commented 5 years ago

Thanks for reporting the issue @Ariandr. One remote image definitely should work, no need for any special treatment here. I tried one image, but could not reproduce your issue, please see the project attached. Does this demo app work for you if you use your image url?

AukOneImageDemo.zip

Ariandr commented 5 years ago

@evgenyneu I will check and let you know.

Ariandr commented 5 years ago

@evgenyneu Demo works, but I found the cause of the issue. If I try auk.show(url: ..) when bounds are (0.0, 0.0, 0.0, 0.0), the image doesn't get loaded. It prints Auk WARNING: scroll view has zero width. and Moa doesn't even start loading the image.

However, if I make sure bounds != .zero, it works.

 override func layoutSubviews() {
        super.layoutSubviews()

        if bounds != .zero {
            show() // works for 1 image
        }
    }

    // my scrollView is a subview of the view where I override layoutSubviews()

But when I show 2 images when bounds are (0.0, 0.0, 0.0, 0.0), it prints the same warning, but it starts scrolling between them and everything works anyway. I don't need to do that in layoutSubviews().

So, behavior for 1 image and 2+ images immediately added is different. When I try showing only 1 image, I need to make sure I do it when bounds != .zero for it to work, but for 2+ images I can do it at any moment of the view lifecycle and it works.

evgenyneu commented 5 years ago

Very interesting, thanks for investigating! The Auk downloads the images whenever they become visible to the user by calling scrollView.bounds.intersects(page.frame). Hence, the scroll view with zero size may not trigger the download. However, there is a settings.preloadRemoteImagesAround option for preloading neighboring images that are not currently visible. Probably, this option is the cause of the inconsistency that you observe, when it still loads images for a zero-sized scroll view but only when it has more than one image.

The way we could fix inconsistency is by adding a check to ensure the images are not loaded when the size of the scroll view is zero. However, I'm not a big fan of adding changes to the library for the sake of change, since it may lead to unintended consequences and break existing apps. I think this bug relates to an edge case not many people will see, since the scroll view is supposed to have non-zero size in order to work.

Is there a reason your scroll view has zero size when you start loading the images? Can you make its size non-zero before calling show as a workaround?

Ariandr commented 5 years ago

Yes, it is created dynamically in code and is nested in other views in the controller. So, I call auk.show immediately after adding the scroll view as a subview and setting up the constraints to it. But there is a delay between setting up the constrains and actual frame changing. Anyway, that's not a problem, I know 2 workarounds and they work for me. I just wanted to understand the intended behavior and in case it was an issue, then it might have been fixed. But since it's not an issue, I agree there is no point to change existing behavior. Thanks for explanations🙂 The only thing which may be improved, as for me, is the warning message Auk WARNING: scroll view has zero width. Maybe there is a point in mentioning that the image won't be shown and downloaded unless show function is called on a non zero bounds scroll view.

evgenyneu commented 5 years ago

Yes, it is created dynamically in code and is nested in other views in the controller. So, I call auk.show immediately after adding the scroll view as a subview and setting up the constraints to it. But there is a delay between setting up the constrains and actual frame changing.

That's actually a valid use case. It wish Auk handled this out of the box by detecting the change of bounds and calling the onScroll method to load the images. I found one way of doing this by adding an observer for bounds. But it feels like a hack to me that can potentially backfire and lead to unexpected behaviour for other users.

For now, I think your method of showing the images in layoutSubviews of the view controller is a reasonably clean work around. Another workaround would be to force the scroll view to do layout immediately after setting up constraints and before adding images to Auk.