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

switchLoadingClass getting called twice for same img #614

Closed
collinsethans opened this issue Mar 9, 2019 · 1 comment
Closed

switchLoadingClass getting called twice for same img #614

collinsethans opened this issue Mar 9, 2019 · 1 comment

Comments

@collinsethans
Copy link

Today we wanted to import lazysizes to our project. We are using it to load images and our each img is:
<img class="lazyload" data-src=".." alt="..." />

The images are only in vertical scroll mode (haven't tried with horizontal scroll yet). We are using your latest version, v4.1.6.

During testing, I notice that switchLoadingClass is getting called twice.
One call is due to:
addRemoveLoadEvents(elem, rafSwitchLoadingClass, true); <- line 486

Second call is due to:
switchLoadingClass(event) in raF(...) <-- line 520

This 2nd call can be stopped (and it works fine) if I pull out the conditional check outside raF(). Seems that during the raF execution, both elem.complete and (elem.naturalWidth > 1) are 'true'.

This is what I did:

if( !firesLoad || (elem.complete && elem.naturalWidth > 1)){
    rAF(function(){
            if(firesLoad){
                resetPreloading(event);
            } else {
                isLoading--;
            }
            switchLoadingClass(event);
    }, true);
}

Not sure if the 2nd call is necessary, but to us it seems redundant. Hence this report.

@aFarkas
Copy link
Owner

aFarkas commented Mar 20, 2019

There are browsers where a load event doesn't happen under specific circumstances. This is a good workaround for that. In your linked issue you call out lazySizes it would have performance issues because of it.

I assume you simply don't understand why I do things I'm doing. And then overact by claiming with things that are simply untrue. This is also shown by your other not linked issue where you simply do not understand the difference between throttle and debounce and start to open issues because it must be a bug because you don't understand it.

If you know a little bit about JS and how the browser renders you would know that executing the following code inside of one rAF is basically transformed into just one call:

element.classList.add('lazyloaded');
element.classList.add('lazyloaded');
element.classList.add('lazyloaded');
element.classList.add('lazyloaded');

This is for example also explained here.

It might not be nice but never ever produces any performance issues otherwise I would have worked it out another way. So why do you claim there are performance issues with lazySizes?

On the other hand your linked and here advertised yall is producing obvious DOM leaks in dynamic websites, is binding an active touchmove handler (without any need) and is mutating the DOM inside of a requestIdleCallback. If you would know a little bit about how the browser renders you would know that rIC gives you a place inside of the frame life cycle that is perfect for layout reads and layout neutral stuff but extremely bad for layout writing stuff and that developers should never do things like this because this invalidates the layout for the upcoming frame. But of course it sound somehow right so you think you would understand and it think it would be a good thing... wrong.

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

Successfully merging a pull request may close this issue.

2 participants