Check if image is fully loaded

[aderaaij]

This is a quick and dirty pull request to check if images have already loaded. This will not pass your tests yet, and doesn't yet take in account srcset either, but I wanted to get the ball rolling!

A few thoughts.

I've modified the markAsLoaded function to simply implement a waiting for images function, data-loaded is only added after the image is fully loaded. I can imagine that there might be circumstances where you want to do something as soon as the image is added, so maybe it's an idea to add another data-attribute especially for when the image is fully loaded.

I also made a solution for srcset but it isn't very elegant yet. It uses p-wait-for, which only adds a couple of bytes to the total. I didn't include this solution in my pull request yet, I'd like to discuss it first!

function markAsLoaded(element) {
  if (element.dataset.srcset) {
    pWaitFor(() => {
      if (element.currentSrc !== '') {
        return true
      }
      return false
    }).then(() => {
      const imgLoad = new Image()
      imgLoad.onload = () => {
        element.dataset.loaded = true
      }
      imgLoad.src = element.currentSrc
    })
  } else {
    const imgLoad = new Image()
    imgLoad.onload = () => {
      element.dataset.loaded = true
    }
    imgLoad.src = element.dataset.src
  }
}

[jimblue]

@aderaaij this could help don't you think?

https://github.com/marlospomin/turtle/blob/master/src/turtle.js

[aderaaij]

@jimblue Yeah it's done in a nice way, somewhat similar actually! Only it doesn't have solutions for srcset included.

[jimblue]

Indeed @aderaaij... does the actual PR work for background image on your side? I doesn't here

[aderaaij]

Good question! No it doesn't, I only check the normal src now. Background image shouldn't be too hard though. I'll have a check later on

[jimblue]

Allright. I'll work on this too. 😄

[jimblue]

I was thinking about a nice way to make the loading feeling really smooth:

• observe all .lozad elements
• preload image outside of intersection (for exemple: rootMargin * 2)
• mark image as loaded when reaching intersection (rootMargin * 1)

The benefit is to preloaded images outside viewport but showing the animation when reaching viewport.

What do you think?

[aderaaij]

It's a good idea but I think that would be for users themselves to define. You can override the rootmargin and treshold when calling the script, so everyone can figure out what works smooth for themselves, as long as there is a part in there that tells when the image is fully loaded.

[jimblue]

That's true

[aderaaij]

Added support for background images

[jimblue]

Thanks @aderaaij!!!! I can't wait @ApoorvSaxena have a look to this PR 😝 !

[jimblue]

Hi @aderaaij! Do you now why checks have failed?

[OlehDutchenko]

@jimblue look at Travis log
https://travis-ci.org/ApoorvSaxena/lozad.js/builds/317439884?utm_source=github_status&utm_medium=notification

[jimblue]

oh ok just learn somethin! Thanks @dutchenkoOleg ... Just don't know how to fix this ^^

[aderaaij]

@jimblue in this pull-request I explained I didn't bother passing all the tests as I wanted to discuss this PR first. I don't know too much about tests, but they're failing because they're expecting an image to be returned right away, so they probably also need the 'loading' script included. Oh and there's one about SRC Set too 😅.

But to be honest I think @ApoorvSaxena is pretty preoccupied, and also it's around x-mas time so there might not be happening too much anyway. So you might have to run with the fork for now!

[jimblue]

Hey @aderaaij ! No worries about this, I'm already pretty happy with Lozad!
The PR will come after x-mas for sure. It's good time to stay AFK 🎉

[OlehDutchenko]

@aderaaij PR has conflicts. I had the same problem in my pr #80.
You need to update your fork and rebuild dist scripts for resolving conflicts.

Also, could you please consider processing of an picture element
https://github.com/ApoorvSaxena/lozad.js/blob/master/src/lozad.js#L12

[OlehDutchenko]

Also you need to mark the items that have already started to load.
Otherwise, duplicate method calls for image loading are possible.
As long as the source is being loaded, there is no attribute, and when the check is repeated, the code for downloading will be called again.

Pay attention to these lines of code:

• https://github.com/ApoorvSaxena/lozad.js/blob/master/src/lozad.js#L35
• https://github.com/ApoorvSaxena/lozad.js/blob/master/src/lozad.js#L71

Maybe you need just add new data attribute, when image is loaded.
for example:

• data-loaded="true" - when data-src inserted as src
• data-full-loaded="true" - when source is fully loaded

[aderaaij]

@dutchenkoOleg Thanks for the ideas! I'm completely aware that these don't pass the tests as I just wanted to get some opinion of the library maintainers first. As I say in my opening post:

This is a quick and dirty pull request to check if images have already loaded. This will not pass your tests yet, and doesn't yet take in account srcset either, but I wanted to get the ball rolling!

If I ever hear back about this I will definitely take your suggestions into consideration 👌

[jimblue]

Hi @ApoorvSaxena!

Can we have your input on this to move on and start fixing #49?

Thanks

[ApoorvSaxena]

@jimblue @aderaaij #86 implementation looks better, let's continue with that unless you want to add anything apart from that

[aderaaij]

@ApoorvSaxena Nope I'm totally good with that!

[jimblue]

@ApoorvSaxena yep I'm good with that too !

EDIT: after some test it seems that #86 can't replace this PR as it doesn't check if image is fully loaded...

[dlecan]

This PR is very interested, I need it, how can I help to make it merged?
Thank you.

[danbilokha]

it's shame it was never merged :c