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

Support threshold for X and Y axis #182

Closed
tom10271 opened this issue Feb 21, 2018 · 21 comments
Closed

Support threshold for X and Y axis #182

tom10271 opened this issue Feb 21, 2018 · 21 comments

Comments

@tom10271
Copy link

I have a lot of images placed vertically and some images inside gallery. I set the threshold to 800px and it is working fine vertically but I want to set a threshold horizontally with 2000px. I cannot achieve it by what supported now.

Thank you for your good work

@verlok
Copy link
Owner

verlok commented Feb 21, 2018

Hi @tom10271,

threshold should work also horizontally, what might not work is if you use a slider or something different from the "natural" browser scroll, meaning either the browser window or a container with the overflow property set to scroll or auto.

Are you using a slider perhaps?

If you're using a slider, I'd suggest you to use slick slider that also has the "lazy loading" feature out of the box.

@tom10271
Copy link
Author

I know it is working for X and Y axis but I want to set 800 for Y and 2000 For X. I am using picture tag which many plugins are not natively supported. The reason why I want to set Y to 800 but not 2000 is because when offset for Y is 2000 it is more or less the same as no lazy load, there will be a lot of images got selected

@verlok
Copy link
Owner

verlok commented Feb 25, 2018

Interesting. I would give the possibility to pass an array of 2 elements, x and y, on the threshold option. What do you think?

@tom10271
Copy link
Author

tom10271 commented Feb 26, 2018

I think is better as it is clearer

{
     threshold: {
          x: 2000,
          y: 800
     }
}

@verlok
Copy link
Owner

verlok commented Feb 26, 2018

Yes you’re right, I’d go for that.
If you’d like to do it your self and open a pull request, the file is src/lazyload.js and the point is where you find the following.

this._observer = new IntersectionObserver(onIntersection, {
            root: settings.container === document ? null : settings.container,
            rootMargin: settings.threshold + "px"
        });

@tom10271
Copy link
Author

Oh I am using 8.6.0. We need to support major browsers

@verlok
Copy link
Owner

verlok commented Feb 26, 2018

Of course. The fix would need to be implemented in both major versions.

@verlok
Copy link
Owner

verlok commented Feb 27, 2018

Hey @tom10271,

I implemented the differentiable threshold here (the code is already working) but then, when I was about to create a demo for it, I realized that I can hardly imagine a scenario where you need to set a different threshold for both directions.

I mean, the scrolling direction is usually only one in each container, right? A container is hardly scrollable in both directions and, even it it was, why would someone set two different values for the threshold?

You started this issue describing your problem with the following sentence:

I have a lot of images placed vertically and some images inside gallery. I set the threshold to 800px and it is working fine vertically but I want to set a threshold horizontally with 2000px. I cannot achieve it by what supported now.

So it seems to me that you have a main vertical scrolling container, the browser window perhaps, and some other horizontally scrolling containers. Did I understand correctly?

If this was the case, you would need to create multiple instances of LazyLoad: one for the main window, and one for each scrolling container. You could pass different options to each instance: the main LazyLoad would have threshold: 800, and the horizontally scrolling containers could have threshold: 2000.

Example:

var mainLazyLoad = new LazyLoad({
  threshold: 800 // this threshold works vertically since the browser window is scrolling vertically
});

var horizontalContainers = document.getElementsByClassName("horizontal");
for (hContainer of horizontalContainers) {
  new LazyLoad({
    container: hContainer,
    threshold: 2000 // this threshold works horizontally since the container is scrolling horizontally
  }) 
}

Did I get it right?

@verlok
Copy link
Owner

verlok commented Feb 27, 2018

Also I think your issue is very similar to #172 that was opened in January.
Please read the resolution comment, take a look at this pen and let me know if this is your case.

@tom10271
Copy link
Author

tom10271 commented Feb 28, 2018

My case is not the same as the one you show and the one in #172.

Images inside gallery are large and listed horizontally. Yes I can define a lazyload instance for gallery with high threshold but it makes browser start to load images much earlier than it should. So to deal with this issue I need a small vertical but high horizontal threshold even it is specifically for my gallery only.

Here is one of the example page that even I set for example 2000 as threshold, the situation is still not ideal:
https://hypebeast.com/zh/2018/1/hypebeast-interview-be-a-model-kol-kiwi-egghead

@verlok
Copy link
Owner

verlok commented Mar 2, 2018

Hi @tom10271,
thank you for sharing the website.

I can see you're using a gallery library (Flickity) on your website.
The fact is that when you use a gallery library, you don't have to deal with the LazyLoad options, because LazyLoad only manages scroll and the gallery libraries don't use scroll, they move the object in the page.

I did a quick research and I've found how to lazily load the images using Flickity: you must use Flickity's lazyLoad option. More info here: https://flickity.metafizzy.co/options.html
This would solve the horizontal loading, making the invisible images load only when the user requests it.

But this has nothing to do with my LazyLoad script, so I'm going to close this issue.
Let me know if you have more questions.

@tom10271
Copy link
Author

tom10271 commented Mar 3, 2018 via email

@verlok
Copy link
Owner

verlok commented Mar 4, 2018

Hi again,
forking the project and try to fix it yourself won’t work for the reason I tried to explain to you above: scroll events and carousels are two very different things.
If you want to fix it yourself, you would need to fork Flickity.
Good luck, anyway.
If you have any questions, I’m here.

@tom10271
Copy link
Author

tom10271 commented Mar 5, 2018

The nature of Flickity is it layouts all images in the direction you want but not injecting images on the fly. Flickity supports lazyload but only for img tag only. Yes my web page only scroll vertically but images are layout horizontally, when I set threshold to 800 with my picture tag used Flickity gallery it is not updating quick enough when user switch gallery pages.

Anyway, you are the most responsive open source developer I have ever met. I really appreciate. Thank you again

@tom10271
Copy link
Author

tom10271 commented Mar 5, 2018

Currently I will call lazyloadInstance.udpate() when user switch to next images to archive lazy load for picture tag images. CEO complains it flicks, obviously the lazy load threshold is not enough.

@verlok
Copy link
Owner

verlok commented Mar 6, 2018

The update method is only useful when you want to check the scrolling container for new images to manage, it doesn’t do anything else. I didn’t understand if this is your case.

But can I ask you the reason why you’re using the picture tag? I’m asking because if you want to do responsive images, most of the times you can do it simply with the img tag, using the srcset and the sizes attributes. And this would solve your issues with Flickity LazyLoad.

@tom10271
Copy link
Author

tom10271 commented Mar 6, 2018 via email

@verlok
Copy link
Owner

verlok commented Mar 6, 2018

I still don’t understand why you’re using the picture tag. You probably might find this post useful.

@verlok
Copy link
Owner

verlok commented Mar 6, 2018

And this article maybe.

@tom10271
Copy link
Author

tom10271 commented Mar 9, 2018

metafizzy/flickity#172

I think picture tag or img tag with srcset are not the core problem. I can use img tag with srcset too but Flickity is not supporting lazy load with srcset too. We have tried flexslider 2, swipebox, photoswipe and flickity already, they are all imperfect solution but Flickity is working to best so far. We do not want to give a try to another gallery. We are still seeking for solution to solve image lazy load in Flickity gallery

@verlok
Copy link
Owner

verlok commented Mar 12, 2018

I understand the problem you’re having, but you can’t fix it with this LazyLoad script for the reasons I mentioned above.

I think you still didn’t try the ultimate carousel. It’s named Slick slider and I’ve been using it for years. It’s open source here and it’s made by @kenwheeler.

It supports lazy loading of responsive images through the srcset in the data-lazy attribute.

The final solution would be a mix of my LazyLoad and Slick slider:

  1. use my LazyLoad to instantiate a new Slick when it’s close to vertically appear in the viewport
  2. Slick will take care of lazy loading the images horizontally.

I can totally set up a demo on Codepen if you need help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants