First click on lightbox works, but not from other galleries

Question

I have built a lightbox functionality in JavaScript, and I'm now working to get the implementation working when having multiple galleries at the same page, a feature I didn't take into account when developing it.

To achieve the desired effect, I'm using dataset and data-*-attributes extensively (data-index to index, data-total to store how many images are in the .gallery and data-current to store which .gallery that should be displayed in the #lightbox. The .gallery with thumbnail <img>s are wrapped in <figure><a> with a href attribute to the bigger #lightbox img.

These attributes work theoretically, and return the correct values with accompanying code. After page load, when clicking on one of the galleries, the gallery works as expected, being isolated from the others, looping through the gallery images.

The problem arise when closing the lightbox, and clicking on one of the other galleries. In IE 11, I get a Unable to get value of... error in the developer tools on the last document.querySelector line. When outputting the returned values to console.log(), however, I can't understand why it isn't adding the .current class used to show the , because when values are correct and looks like a valid selector (and it still works in the first-clicked .gallery):

This is a heavily shortened version of the JS code. Variables and selectors have also been translated into English so that people here could understand it better. Take an extra good look at the galleryindex, galleryindexarray, thisgallery, thisgallerya and current variables, which is where the functionality around the problem lays.

var lightbox = document.createElement('div'),
    gallerya = document.querySelectorAll(".gallery a"),
    current, img, i, galleryindexarr = [];
for (i = 0; i < gallerya.length; i++) {
    gallerya[i].addEventListener("click", function(e) {
        e.preventDefault();
        var galleryindex = this.parentNode.parentNode.dataset.index,
            thisgallery = document.querySelector(".gallery[data-index='" + galleryindex + "']"),
            thisgallerya = document.querySelectorAll(".gallery[data-index='" + galleryindex + "'] a");
        if(!document.getElementById("lightbox")) {
            lightbox.id = "lightbox";
            document.body.insertBefore(lightbox, document.body.firstChild);
        }
        if(galleryindexarr.indexOf(galleryindex) == -1) {
            galleryindexarr.push(galleryindex);
            for (i = 0; i < thisgallerya.length; i++) {
                img = document.createElement("img");
                img.src = thisgallerya[i].href;
                img.dataset.index = galleryindex;
                lightbox.appendChild(img);
            }
        }
        current = [].indexOf.call(thisgallery.childNodes, this.parentNode);
        document.querySelector("#lightbox img[data-index='" + galleryindex + "']:nth-of-type(" + current + ")").classList.add("current");
    });
}

Here is a test demo site with further visual clarification. Please assist me on this rather small error if you know anything that could help.

As you can see in the picture, [in the full code](http://dd.no/lysboks.js) and probably in your own console [on the demo site](http://dd.no/lightboxtest), they are already logged. As for jQuery, I know it is easier to write, but I've abandoned it for a faster and more pure code. — mnsth, Jul 24 '15 at 20:01
Sorry I am on mobile - I see you did log it. I even read Norwegian :) — mplungjan, Jul 24 '15 at 20:04
Yeah, it's Norwegian, how did you know? :) and could you please help me with this if you have any knowledge? — mnsth, Jul 24 '15 at 20:13
I am Danish, and I would love to help, but I do not have access to a console nor IE11 here — mplungjan, Jul 24 '15 at 20:16
I see, thank you very much. When will you have access to one; in a couple of hours, perhaps? It's not *that* urgent; I'll wait, as long as you can help me later. — mnsth, Jul 24 '15 at 20:20
Could it be the selector returns more than one element so you need to do [0] or make sure you have only one of that name? — mplungjan, Jul 24 '15 at 20:26
Monday could work, too, feel free to post your suggestions then. I don't think so, `document.querySelector` only returns the first element anyway, doesn't it? Besides, the selector is complex, and I know for sure that it would only apply to one of the ``s. — mnsth, Jul 24 '15 at 20:30

score 0 · Answer 1 · edited Jul 25 '15 at 08:48

0

After almost two hours of testing, I happened to detect the problem. There was a problem in the selector as @mplungjan mentioned in the comments, so working in your full code, try to replace the selector in following line (after the two consoles log lines) by the correct selector.

Replace:

 document.querySelector("#lysboks img[data-galleriindeks='" + galleriindeks + "']
 :nth-of-type(" + gjeldende + ")").classList.add("current");

By:

 document.querySelectorAll("#lysboks img[data-galleriindeks='" + galleriindeks + "']").item(gjeldende-1).classList.add("current");

Now you should be able to see the pictures, but the next and previous pictures do not work yet.

To fix navigation (next & previous) buttons, you have to replace the line below in your gå() function to get the correct selector.

Replace :

document.querySelector("#lysboks img[data-galleriindeks='" + document.querySelector
(".galleri[data-gjeldende]").dataset.indeks + "']:nth-of-type(" + destinasjon + ")").
classList.add("current");

By :

document.querySelectorAll("#lysboks img[data-galleriindeks='" + document.querySelector(".galleri[data-gjeldende]").dataset.indeks + "']").item(destinasjon-1).classList.add("current");

And now it should work fine (tested in Google Chrome). I hope that this helped to fix your issue. Good luck, and have sweet weekend.

edited Jul 25 '15 at 08:48

mnsth

2,169
1
18
22

answered Jul 24 '15 at 22:15

Zakaria Acharki

66,747
15
75
101

Thank you very much. I will test it tomorrow to see if it works for me. However, if it does, I'm interested in knowing why that works, and not my code. Both `.querySelector` and `:nth-of-type` should work because it's valid code, isn't it? Have a great weekend, too! – mnsth Jul 24 '15 at 22:27
Yes, in fact the problem is the `querySelector` that return just the first `img` so when you try to select one of them by `:nth-of-type` to add the `current` class it's unable to select it, instead of `querySelectorAll` that return all the `img` elements matching the selector options, so when i select with `index` using `item()` function it's work. – Zakaria Acharki Jul 24 '15 at 22:38
Thanks for your explanation, however, I don't understand - do you know why the images in the first-clicked gallery works, then? I want to become better in JavaScript, so I need to know exactly why it doesn't work. I only wanted to select one element, therefore I thought `.querySelector` was sufficient. – mnsth Jul 25 '15 at 08:30
I've found out why! As we all know, the selector inside `.querySelector` is (and should be) a CSS selector. In my CSS, I tried to apply an outline to every `:nth-of-type(2)` in each `[data-galleriindeks]` - and that only works on the first click, too! It doesn't target the second-clicked and third-clicked gallery, and the outline is only applied to the second img in the first-clicked gallery, just as in JavaScript. Everything I knew about CSS selectors (and `:nth-of-type`), I have to take a second look on. I will now investigate why even the CSS works as it does. – mnsth Jul 25 '15 at 09:26
Surprisingly, it looks like `[attributes]` aren't accounted for when using `:nth-of-type`. So this selector `#lysboks img[data-galleriindeks="2"]:nth-of-type(1)` doesn't apply to the second `img` with `[data-galleriindeks="2"]`, but the second `img` overall with that attribute. And since the gallery clicked first are the first images appended, it works, since the numbering is there, but I might as well just have written `#lysboks img:nth-of-type(1)`, and it wouldn't change anything, because attributes aren't accounted for when counting. That's a shame! – mnsth Jul 25 '15 at 09:43

First click on lightbox works, but not from other galleries

1 Answers1