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

data-srcset remaining as data-srcset when visible #134

Closed
TokyoWeb opened this issue Oct 18, 2018 · 5 comments
Closed

data-srcset remaining as data-srcset when visible #134

TokyoWeb opened this issue Oct 18, 2018 · 5 comments
Labels

Comments

@TokyoWeb
Copy link

Hi ! First i would like to thank you for Lozad. So powerful and such a small size. I love it.

Nonetheless after two months using and placing like 500 responsive images in my site, I just realized I must be doing something wrong or there is an issue when using "data-srcset". The "data-src" is loaded as "src" when visible, but the data-srcset remains always "data-srcset" then the browsers are only loading the image that I place in "data-src" and ignoring all the responsive images inside "data-srcset".

I first realized this when checking a performance test (yellowlab tools reported same weight for mobile and desktop in an image that I thought was below the fold, and then I realized my issue, it should have loaded different images for mobile, tablet and desktop). Then I checked on Safari, Chrome, Firefox and when disabling cache always only one image is shown.

If I load the images as "picture tag", no issue. I did a couple of codepen with my code.

Example using picture tag https://codepen.io/tokyoweb/pen/MPVVbK working as expected.

Example using data-srcset https://codepen.io/tokyoweb/pen/WazZLP which is the code I use in like 500 images in my site :(

No matter what I do, the responsive images inside "data-srcset" remain like that, because I think I must be doing some error and "data-srcset" doesn't change to "srcset" when visible as it does happen with "data-src" that become "src" when visible.

Thanks and I am sure it must be me doing something wrong.

<img class="lozad" data-src="image-w250.jpg" data-srcset="image-w446.jpg 446w, image-w606.jpg 606w, image-w700.jpg 700w" />

@ApoorvSaxena
Copy link
Owner

Please find the comments inline.

Hi ! First i would like to thank you for Lozad. So powerful and such a small size. I love it.

Thanks for the kind words.

The "data-src" is loaded as "src" when visible, but the data-srcset remains always "data-srcset" then the browsers are only loading the image that I place in "data-src" and ignoring all the responsive images inside "data-srcset".

As per the codepen implementation, it should have worked, I'll be checking the codebase once, not really sure if it's related to #99 (for which I have an open PR waiting to be reviewed and merged #116 )

@ApoorvSaxena
Copy link
Owner

@TokyoWeb just found that you've overwritten load function which seems to be copied from our demo page

// Initialize library to lazy load images
var observer = lozad('.lozad', {
    threshold: 0.1,
    load: function(el) {
        el.src = el.getAttribute("data-src");
        el.onload = function() {
            toastr["success"](el.localName.toUpperCase() + " " + el.getAttribute("data-index") + " lazy loaded.")
        }
    }
})

the new overridden load function definition only handles src case and not srcset or any other case that original load function handles, thus don't override load function in your actual code and you should be good to go 👍

@TokyoWeb
Copy link
Author

Hi! Thank you ApoorvSaxena so much to point my mistake. Luckily then the code for my images is fine, the mistake was with the load code.

If someone has same issue, just do as usual the data-srcset as well as the js code. I left here an example working perfectly fine. https://codepen.io/tokyoweb/pen/qLdgMJ Just reload the page / browser window at less than 450px, then 620px and over 750px width to see how new image from data-srcset as being loaded according to web page width.

@TokyoWeb
Copy link
Author

ApporvSaxena, thanks again for your help. I am now curious how to add a class when lozad loads the image from data-srcset ?

If I use this code, it does add the fade effect but ignores the data-srcset images and just loads the image from data-src.

// Initialize library
lozad('.lozad', {
load: function(el) {
el.src = el.dataset.src;
el.onload = function() {
el.classList.add('fade')
}
}
}).observe()

I saw your codepen from css-tricks ( https://codepen.io/ApoorvSaxena/pen/oGgxJr ) in which you add a fade effect by adding a class called fade when it loads the image. Can I also do the same with data-srcset ?

Until now I only managed to make data-srcset to work when using only the following code...

const observer = lozad(); // lazy loads elements with default selector as '.lozad'
observer.observe();

but then how to add an css effect / add a class when lozad loads one of the responsive images from data-srcset ? It seems I need the first code to load lozad ( and fade class), but then I lose the data-srcset images and only loads the image in the data-src image.

@daniellzl
Copy link

@TokyoWeb

You need to add el.srcset = el.dataset.srcset; after el.src = el.dataset.src; in your custom load method for your fade effect to work with srcset.

Take a look at the original load method (https://github.com/ApoorvSaxena/lozad.js/blob/master/src/lozad.js) if you need to see how it handles all the different image attribute cases.

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

No branches or pull requests

3 participants