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

callback_exit() is not being called on non-image elements #468

Closed
starfishpatkhoo opened this issue Jun 24, 2020 · 6 comments
Closed

callback_exit() is not being called on non-image elements #468

starfishpatkhoo opened this issue Jun 24, 2020 · 6 comments

Comments

@starfishpatkhoo
Copy link

starfishpatkhoo commented Jun 24, 2020

I have been using IntersectionObserver for other purposes. Then I decided to add LazyLoad to lazyload the images. Since LazyLoad also uses IO, I thought I might as well switch all my own IO event handlers/hooks to use LazyLoad. Everything works fine, except for callback_exit(). As it turns out, the IO detection on leaving works fine, it is just that it happened that I was using this on some other elements that have nothing to do with images. And so onExit() would ignore the event.

To reproduce, just create a div and have LazyLoad watch it with callback_enter() and callback_exit().

<div class="my-class">Some stuff</div>
var ll = new LazyLoad({
	elements_selector: ".my-class",
	callback_enter: callback_enter_fn,
	callback_exit: callback_exit_fn,
});

You will see it enters, but never exits :)

The fix is below. But I'm pretty sure there is a more "proper" way to do this and I hesitate to submit a PR without knowing the true effect of such a change.

--- lazyload.intersectionHandlers.js	2020-05-21 13:18:18.000000000 +0800
+++ lazyload.intersectionHandlers.NEW.js	2020-06-24 12:16:48.984638000 +0800
@@ -12,7 +12,8 @@
 };
 
 export const onExit = (element, entry, settings, instance) => {
-    if (hasEmptyStatus(element)) return; //Ignore the first pass, at landing
+    if ((element.tagName === "IMG") && (hasEmptyStatus(element))) return; //Ignore the first pass, at landing
     cancelLoadingIfRequired(element, entry, settings, instance);
     safeCallback(settings.callback_exit, element, entry, instance);
 };

I understand that this was never really LazyLoad's intention, I just thought I would save some code and avoid duplication in my own project.

Version: 16.1
Browser: Chrome (but any IO enabled browser really)

EDIT: Issue also exists in Version 17.0

@verlok
Copy link
Owner

verlok commented Jun 25, 2020

Hey @starfishpatkhoo, thanks for you thorough debugging.

If we applied the fix you are suggesting, you would get a lot of callback_exit called at landing, and it wouldn't work for iframes and videos.

I think the fix is to apply a status entered (I did this before, but then I deleted it) to all the observed elements, so the hasEmptyStatus(element) doesn't return true and the callback is then called.

export const onExit = (element, entry, settings, instance) => {
    if (hasEmptyStatus(element)) return; //Ignore the first pass, at landing
    cancelLoading(element, entry, settings, instance);
    safeCallback(settings.callback_exit, element, entry, instance);
};

Let me try this way.

@verlok verlok closed this as completed in 2cd50f3 Jun 25, 2020
@verlok
Copy link
Owner

verlok commented Jun 25, 2020

Hello @starfishpatkhoo ,
this bug should be fixed now in 17.0.1.

Would you be able to double check?

@verlok
Copy link
Owner

verlok commented Jun 28, 2020

@starfishpatkhoo please let me know if this worked for you.

@starfishpatkhoo
Copy link
Author

Sorry @verlok working on some other high priority issue a.t.m. .. I will be able to get back to this and the other one on tuesday.. apologies :)

@starfishpatkhoo
Copy link
Author

Works great, thanks @verlok! Yes, that's a good way to go about it to add a "entered" status, safer than checking the type of element. :)

@verlok
Copy link
Owner

verlok commented Jun 29, 2020

Hey @starfishpatkhoo ,
I’m glad this solved!

Thank you again for reporting the bug.
Have a nice day!

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

No branches or pull requests

2 participants