user11706116
user11706116

Reputation:

Why is my JavaScript code executing only on second click?

I have been toying with this forever now. I can't seem to find out why the lightbox is only executing after I've clicked it twice. The first click, it just pops the image up in a new tab.

I've already tried using e.preventDefault (which did nothing except keep the image from popping up in a new tab after the first click).

$(document).ready(function() {

    $('a[class^="fomod-"]').on('click', function() {
        var fomodClass = $(this).attr('class');
        var fomodGallery = $('.' + fomodClass).simpleLightbox({
            loop: false
        });
    });

});

What I'm ultimate trying to do is watch the DOM for any clicks on links that have the "fomod-*" class and, if clicked, get the exact class of the element that was clicked. With that, the lightbox pops up and only shows the other images with that same exact class as a gallery.

Upvotes: 3

Views: 141

Answers (1)

Tyler Roper
Tyler Roper

Reputation: 21672

The Issue

.simpleLightbox() initializes the lightbox. That means your first click adds simpleLightbox to your page, allowing all following clicks to actually trigger it.

You need to do the initialization when the page loads. Now you could do something like...

$('a[class^="fomod-"]').each(function() { ... })

But that has a few drawbacks.

  1. It won't find elements where fomod isn't the first class, ie class="other-class fomod-one".
  2. If you had class="fomod-one other-class", your internal selector won't work because the concatenation would result in $(".fomod-one other-class").
  3. You'd be continuously re-initializing simpleLightbox on the same elements repeatedly, which I'm not sure if the plugin is setup to handle or not.

Solution 1 - Data Attributes

data attributes allow us a bit more flexibility on how we select our elements. Also, fetching data attributes in JavaScript is supported in both vanilla JS (using dataset) and jQuery (using .data()).

<a data-fomod="Gallery1"></a>
<a data-fomod="Gallery1"></a>
<a data-fomod="Gallery2"></a>
$(document).ready(function() {

  const galleries = $("[data-fomod]")
    //Get array from elements
    .get()
    //Attach lightbox to each unique gallery on the page
    .reduce((output, {dataset}) => {
      let galleryName = dataset.fomod;
      return output[galleryName] 
        ? output 
        : {...output, [galleryName]: $(`[data-fomod="${galleryName}"]`).simpleLightbox({loop: false})}
    }, {});

});

This approach gives us three things over the initial approach:

  1. It doesn't restrict the use of classes.
  2. It attaches simpleLightbox to each gallery just once.
  3. It stores the galleries individually by name in the galleries object. For example, if you wanted to tell Gallery1 to go to the next slide, you could do galleries["Gallery1"].next().

Solution 2 - Using Classes More Appropriately

As you'd mentioned in the comments, your environment doesn't provide great support for data- attributes. Instead, we can use classes, we just have to be a bit more considerate. I'll use two classes here - one to flag this as a lightbox element ("fomod"), and another to associate the gallery ("fomod-GalleryName").

You may be wondering why that "flag" fomod class is necessary. Why not just use fomod- and use the ^= selector? As mentioned above, what if fomod- is the second class behind my-other-class? The selector won't find the element.

(There are ways around this but that's opening a can of worms.)

This approach, although just slightly more involved, achieves all the same benefits that the data attribute solution does.

<a class="fomod fomod-Gallery1"></a>
<a class="fomod fomod-Gallery1"></a>
<a class="fomod fomod-Gallery2"></a>

Without comments

$(document).ready(function() {
  const galleries = $(".fomod")
    .get()
    .reduce((output, elem) => {
      let galleryName = [...elem.classList].find(c => c.startsWith('fomod-'));
      if (!galleryName) return;
      galleryName = galleryName.split("-")[1];

      return output[galleryName] 
        ? output 
        : { ...output, [galleryName]: $(`.fomod-${galleryName}`).simpleLightbox({loop: false})}
    }, {});
});

With comments

   $(document).ready(function() {

      const galleries = $(".fomod")

        //Get array from elements
        .get()

        //For each fomod element...
        .reduce((output, elem) => {

          //Get the classes on this element, and find one that starts with "fomod-"
          let galleryName = [...elem.classList].find(c => c.startsWith('fomod-'));

          //Didn't find one. Skip this element
          if (!galleryName) return;

          //Remove the "fomod-" part so we're left with just the gallery name
          galleryName = galleryName.split("-")[1];

          //Have we already initialized this gallery?
          return output[galleryName]  

            //Yup. Do nothing.
            ? output              
    
            //Nope. Initialize it now.
            : { ...output, [galleryName]:  $(`.fomod-${galleryName}`).simpleLightbox({loop: false})}

        }, {});

    });

Upvotes: 6

Related Questions