futtta / autoptimize

Official Autoptimize repo on Github
https://autoptimize.com/pro/
GNU General Public License v2.0
282 stars 87 forks source link

lazyload skip first n images does not work on <picture/ <source elements #381

Open futtta opened 2 years ago

futtta commented 2 years ago

see https://wordpress.org/support/topic/lazy-loading-bug/

erikyo commented 6 months ago

Ciao @frutta i did a npm module that is not affected by the common LCP issues and can lazyload videos and backgrounds, i made it following the westonruter advices to get a good results lighthouse rankings (see the slack thread)

https://www.npmjs.com/package/auto-lazyload

Feel free to test it and use as you want if you think that is better than lazysites (that is a bit outdated, isn't it? ๐Ÿ˜‰) as long your plugin is free as it has always been :)

erikyo commented 6 months ago

if needed i can explain how it work in detail but TLDR:

futtta commented 6 months ago

interesting @erikyo .. is this in use already somewhere? standalone or as part of another plugin? would love to see feedback on it from other users :)

erikyo commented 6 months ago

Is this in use already somewhere?

You can disable the lazyload script adding ?no-lazy to the url (e.g. link)

it's a script I made a couple of weeks ago because some customers complained about the performance of some landing pages we made with Gutenberg, which are beautiful but have some critical points that I solved with the first script I posted in the slack conversation with Weston Router. The critical points are about backgrounds and videos that we use intensively but as I said with some drawbacks like the decreased performance

standalone or as part of another plugin?

it's a simple js script, as vanilla-lazyload or the lazySites and actually can be used as iife (as in the website i shared) or as a npm module, anyhow in the wp context i believe it's better the iife version since it's a very tiny script and we wont leverage of the cool stuff of the es modules like the tree-shaking

would love to see feedback on it from other users :)

Me too but somehow i have to propose it otherwise no one will even know it exists. I hope someone who passes by here will try that module and give a feedback.


EDIT: Since some functions were missing in the 1.0.4, at least compared to the most common lazyload scripts, I decided to "fill the gap", and now the 1.1.0 i added a small (but powerful) api and a "real" and tree shakeable esm version of the module. Written in typescript and bundled with recent tools it weights only 1Kb. I addition the script is strictly typed (and the others are not), so AFAIK they should harbour glitches and inconsistencies.

With the last version you can avoid issue like this one in the demo page that holds a slider/swiper script dynamically loaded (e.g. await import('')). Since the slider is replacing the dom elements with a copy that swiper made before they were 'unveiled' and 'unobserved' the displayed images are broken

This issue now can be fixed easily using autolazy.update('.swiper-wrapper img') that will add observe again the elements added by swiper to the intersection observer or eventually with document.querySelectorAll('.swiper-wrapper img').forEach(item=> autolazy.unveil(item)) Just a clarification, normally there is no need to add the new elements to the intersection observer because the mutation observer does it, but this is a special case

This is the gain that can be achieved (obviously on pages with only images the gain is marginal since wp already deal with that using loading="lazy")

erikyo commented 6 months ago

I am writing to you after seeing on slack #performance that your plugin has been 'named' because of lazysites (see. this file shared there)

If you really don't like the futuristic implementation i made (joking๐Ÿ˜‰), I think other implementations were better, e.g. vanilla lazyload that supports backgrounds, has been written in modern js, smaller/faster etc. (ps i'm a contributor so I might be biased ๐Ÿ˜…). And, if I'm i'm not wrong, it's also a drop-in update and you don't need to change any class or php code to make it work.

futtta commented 5 months ago

re. background images, the issue is not with

<div style="background-image:url(lazy.jpg)"></div>

which AO+lazysizes can indeed do, but rather with

<div class="xyz"></div> and then elsewhere in (linked) CSS having .xyz{background-image: url("lazy.jpg");

where the problem is that AO (currently) does not "know" .xyz has a background-image (AO does not look for relation between HTML & CSS (or JS) so it does not alter the HTML or CSS meaning lazysizes has nothing to pick up here :-/

erikyo commented 5 months ago

@futtta you can hook to the event "beforeLazyUnveil" with a function like this one:

document.addEventListener( 'lazybeforeunveil', (e) => {
  const bg = e.target.getAttribute('data-bg');
  if (bg) {
    e.target.style.backgroundImage = `url(${bg})`;
  }
});

this way it should work exactly the same as for images and you will not need to create a css class for it

futtta commented 5 months ago

the issue here is the div does not have a style attribute with the image as background-image, so AO doesn't "know" there is a data-bg to be added, as that URL is set in the linked CSS-file, not in the HTML :)

erikyo commented 5 months ago

ah yes, you are right... i that case would not work ๐Ÿ˜ข.

However I believe that gutenberg uses the style, especially in that critical case of the cover. In that case it could help a lot because they are often very large images