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

use_native breaks lazy loading of videos #527

Closed
saschaeggi opened this issue Jun 4, 2021 · 12 comments
Closed

use_native breaks lazy loading of videos #527

saschaeggi opened this issue Jun 4, 2021 · 12 comments

Comments

@saschaeggi
Copy link

saschaeggi commented Jun 4, 2021

Describe the bug

// Instance using native lazy loading
const lazyContent = new LazyLoad({
  use_native: true // <-- there you go
});

When the native option is used, videos won't lazyload, it looks like those will get ignored. data-src doesn't transform to src, nor gets the loaded class attached.

To Reproduce

  1. Set use_native to true
  2. Add a video with <source data-src="...">

Video won't load at all. If use_native is set to false, it works.

Expected behavior

Allow videos to work together with use_native

LazyLoad version

Version 17.3.2

Desktop (please complete the following information):

All browsers

Smartphone (please complete the following information):

All browsers

@verlok
Copy link
Owner

verlok commented Jun 6, 2021

Hey @saschaeggi ,
thank you for opening this issue.

As you can read in the API documentation, the use_native option only works with <img> and <iframe>.
This is because the loading="lazy" attribute works only on those <img> and <iframe> across all browsers. See browser support.

Are you suggesting that this LazyLoad library should detect if the browser is supporting native lazy loading for videos, and in case do the same thing it does for images and videos?

@verlok
Copy link
Owner

verlok commented Jun 6, 2021

Or are you suggesting that this LazyLoad library should just copy the data-src into src for videos, making them load lazily on browsers that support native lazy loading it, and eagerly load them on other browsers?

@verlok
Copy link
Owner

verlok commented Jun 6, 2021

With the current version of the script, the solution you should adopt is to use two different instances of LazyLoad, one for videos, one for the rest.

// Instance using native lazy loading
const lazyContent = new LazyLoad({
  elements_selector: "img.lazy, iframe.lazy",
  use_native: true 
});

// Instance without native lazy loading
const lazyBackground = new LazyLoad({
  elements_selector: "video.lazy",  
  // DON'T PASS use_native: true HERE
});

Now if I think of an enhancement of the script with an auto-detection of the native lazy loading for video, the question is:

What should the script do when it runs on a browser that doesn't support native lazy loading for videos, and the user passed use_native: true?

  1. Should it just load all the videos eagerly? (wouldn't this be seen as a bug?)
  2. Should it load them lazily, but delegate it to JS? (that would create a misunderstanding of the use_native option, and would require to fork much of the code)

If you ask me, I'd probably go for option 1.
And the way users could have native video lazy loading only where supported is to have the detection logic out of the script.

In other words, the script user should detect if the browser supports native lazy videos, and if yes, it passes the video.lazy in the elements_selector string.

For example:

const supportsNativeLazyVideo = () => "loading" in HTMLVideoElement.prototype;

if (supportsNativeLazyVideo()) {
  // Single instance using native lazy loading on images, videos and iframes
  const lazyContent = new LazyLoad({
    use_native: true 
  });
}
else {
  // Double instance using native lazy loading on images and iframes, 
  // and JS driven lazy loading on videos
  const lazyImgIframe = new LazyLoad({
    elements_selector: "img.lazy, iframe.lazy",
    use_native: true 
  });
  const lazyVideo = new LazyLoad({
    elements_selector: "video.lazy"
  });
}

What do you think @saschaeggi?

@saschaeggi
Copy link
Author

saschaeggi commented Jun 6, 2021

Hey @verlok
I'm already using a similar technique to detect if the browser supports native lazy loading for images:

const nativeImageLazyLoad = 'loading' in HTMLImageElement.prototype;

myLazyLoad = new LazyLoad({
  use_native: nativeImageLazyLoad,
  ...
});

So yes, your idea of using two instances could be the solution to my problem 🙂

@verlok
Copy link
Owner

verlok commented Jun 6, 2021

Hey,
thanks for your quick reply.

Great, I'm already proceding that way.
I'm updating the code and the native_lazyload_conditional.html demo, where I've included videos.

Stay tuned.

@saschaeggi
Copy link
Author

Pleasure is all mine ☺️

@verlok
Copy link
Owner

verlok commented Jun 6, 2021

Solved in 17.4.0! Just released.

@verlok
Copy link
Owner

verlok commented Jun 7, 2021

@saschaeggi are you satisfied with the new version?

@saschaeggi
Copy link
Author

Yes works great, thanks a lot for your effort! ☺️

@verlok
Copy link
Owner

verlok commented Jun 9, 2021

Hey @saschaeggi,
I’m glad it solved!

If you’re happy with my support, the documentation and the effort I’ve been putting on this project in the latest years, I hope you might want to buy me a coffee to show your appreciation. Or sponsor me, if you're a fan.

Open-source software is great for everyone, but it takes passion and time to grow and evolve.

Thank you for thinking about it.

Have a nice day!

@saschaeggi
Copy link
Author

I know exactly how hard it can be to maintain an Open Source project, as I maintain multiple of them myself.
Enjoy your coffee! 😉☕️

@verlok
Copy link
Owner

verlok commented Jun 11, 2021

Thanks, @saschaeggi
You rock.

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