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

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

Open
futtta opened this issue Mar 15, 2022 · 9 comments
Open

Comments

@futtta
Copy link
Owner

futtta commented Mar 15, 2022

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

@erikyo
Copy link

erikyo commented Apr 9, 2024

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
Copy link

erikyo commented Apr 9, 2024

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

  • the script is loaded at the beginning of page load (and didn't wait as the common lazyload the page load event)
  • I use the mutation observer to check the element during the DOM construction checking one by one the elements added, if the element that is entering is an image video o something that supports the lazyload i will cancel the request for that image
  • the script didn't force the reflow of the page since i'm using only the intersection observer to know the position of the Elements

@futtta
Copy link
Owner Author

futtta commented Apr 13, 2024

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
Copy link

erikyo commented Apr 13, 2024

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
Copy link

erikyo commented Apr 16, 2024

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
Copy link
Owner Author

futtta commented May 9, 2024

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
Copy link

erikyo commented May 9, 2024

@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
Copy link
Owner Author

futtta commented May 9, 2024

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
Copy link

erikyo commented May 9, 2024

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

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

No branches or pull requests

2 participants