search

felixhagspiel / jsOnlyLightbox

Responsive Lightbox in plain JS. No need for jQuery.
MIT License
157 stars 49 forks source link

Extend push method for thumbnails attribute #29

Closed soulchainer closed 7 years ago

soulchainer commented 8 years ago

Extend push method for thumbnails attribute. I did this because, when you dinamically load images, the actual implementation only add the images to the lightbox... and not add the click handlers to the extra images.

So, in my case (for explaining a real one), I have a gallery, populated with thumbnails, that has infinite scroll. New thumbnails are fetched via AJAX and displayed in the gallery and, when fetched, I update the lightbox with lightbox.thumbnail.push, but... that only updates the images that are showed in the lightbox.

If I want to display the lightbox clicking in one of the new fetched images, it won't work... because they haven't handlers attached to the click event. So, I had to rerun lightbox.load() every time I fetched a new page of images. That doesn't seem the correct way and even more, that's against the documentation of lightbox.load():

Has to be called once on the box-object

So, I did this. It's simple and works. And only affects to the lightbox.thumbnails attribute.

Maybe it should (or could) be done differently, I don't know. What do you think about it? The only thing I know is that to be forced to call lightbox.load() every time I fetch a new bunch of images doesn't seems the correct way.

I also wasn't so sure where to place it, so I placed at the end of the script, in Public methods.

Note: obviously, minified version should also change. I not touched it just in case you want to minify it in some specific way.

felixhagspiel commented 8 years ago

Valid point. I will look into this

luciano-im commented 7 years ago

Hi, in my project im loading images via ajax and adding them to the thumbnails array as the documentation says, but I'm having the same problem described here, when I click on an image it has not attached the event and it does not open the lightbox. This is solved?

Thank you very much for your time.

felixhagspiel commented 7 years ago

fixed in version 0.5.6. Had some merge conflicts because it took me so long to look into this, thats why I did not accept the PR. But should be fine now. Thanks @soulchainer !