Skip to content

Bug: intersection observer entries not being correctly removed, causing some images to not load #13

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.

Sign up for GitHub

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

Jump to bottom
Closed
anthonygore opened this issue Oct 19, 2017 · 2 comments
Closed

Bug: intersection observer entries not being correctly removed, causing some images to not load #13

anthonygore opened this issue Oct 19, 2017 · 2 comments
Labels
bug v2 (shadowfax)

Comments

@anthonygore
Copy link
Contributor 

loadImage = function(img){
  ...
  elements.splice(elements.indexOf(img), 1);
},

This doesn't work. Firstly, it doesn't remove the observer entry correctly, secondly, if the element is not found i.e. elements.indexOf(img) === -1, the splice removes the last element of the array! The result is that some images don't get loaded.

There's an easy fix, I'll put through a PR.
@anthonygore anthonygore mentioned this issue Oct 19, 2017
Merged
@malchata
Copy link
Owner

I'm just seeing this now, as it must have gotten buried in my notifications. I've noticed this behavior myself, but didn't know exactly where it was failing. I'll test this out and accept your PR once I can verify the fix. Thank you so much, @anthonygore!

malchata added a commit that referenced this issue Oct 22, 2017
@malchata
A number of changes:
e51aa20 
- Implementing @anthonygore's bugfix (#13) where the last image in a set would fail to load, due to my buggy IntersectionObserver code.
- Updated packages.
- Incremented version to 1.1.2.
- Updated the README.
@malchata
Copy link
Owner

FWIW, I did need to still track the number of elements in the array so that the lazy loading behavior could be unbound for users of browsers without Intersection Observer support. So I relied on the filter method to achieve this, which seems to be more reliable than splice.

@malchata malchata assigned malchata and unassigned malchata Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug v2 (shadowfax)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@malchata @anthonygore