6

I'm using jQuery and ajax to open a bootstrap modal window. My code works perfectly when not in a loop, and my content is displayed appropriately in the modal window. But I have five modals on the page, and naturally I'd like to use a for loop so I don't repeat my code five times.

It seems like a no-brainer to write a for loop to execute the code five times, but somehow, there is a problem, and when I put the code into a for loop, the modal content will not show in the opened window.

Here is code that works how I need it, so the modal content from my modal1.php file shows in a modal popup window. Notice that the i variable is used three times; once in the jQuery selector, and twice in the $.ajax(...) area:

var i = 1;

$('#modal' + i).on('show.bs.modal', function(e) {
    var id = e.relatedTarget.dataset.id;
    $.ajax({
        type:'post',
        url:'modals/modal' + i + '.php',
        data:{id:id},
        success:function(data){
            $('#modal' + i).html(data);
        }
    });
});

This code below uses the variable i the exact same way as the code above. However, with the code below, the modal1.php content does not show (nor do any of the other files named 'modal[i].php'). Instead, only a blank dark background appears when they are opened.

for (i = 1; i < 6; i++) {
    $('#modal' + i).on('show.bs.modal', function(e) {
        var id = e.relatedTarget.dataset.id;
        $.ajax({
            type:'post',
            url:'modals/modal' + i + '.php',
            data:{id:id},
            success:function(data){
                $('#modal' + i).html(data);
            }
        });
    });
}

So I don't understand why the $.ajax() area of the code won't recognize the i variable in the for condition, but it will recognize it in the initial jQuery selector $('#modal' + i), which I've tested and found to be true. Is there a way I can write this loop so that the ajax area will recognize the i variable? Or is there a mistake in my code that I'm overlooking?

Incidentally, I've noticed that similar questions have been asked, but they have been downvoted for not being clearly written. I've read them all, and they don't answer the question that I have today.

halfer
  • 19,824
  • 17
  • 99
  • 186
Mark
  • 2,961
  • 1
  • 16
  • 24
  • I _think_ you're running into issues with trying to register your event handler inside of a for loop. Perhaps this might help? https://stackoverflow.com/a/7774659/1729859 – mituw16 Oct 27 '17 at 16:49
  • 2
    The issue is that the i is scoped outside the method, so that it will be the value of 6 when the modal logic executes. You need to make a scoped down variable that is not mutable by the loop. `forEach` or `$.each()` are good for this, which pass in the index to the callback. – Taplar Oct 27 '17 at 16:50
  • 1
    I agree with @mituw16 . Here's another question that goes in to the same thing. https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – Derek Nolan Oct 27 '17 at 16:53

2 Answers2

3

The problem here is that you are (unintentionally) referencing a variable (i) via a closure.

Since scope is dictated by function in JavaScript, wrap your on usage with another (immediately invoked) function so a unique i is saved:

for (i = 1; i < 6; i++) {
    (function() {
        var iModalNumber = i;
        $('#modal' + iModalNumber).on('show.bs.modal', function(e) {
            var id = e.relatedTarget.dataset.id;
            $.ajax({
                type:'post',
                url:'modals/modal' + iModalNumber + '.php',
                data:{id:id},
                success:function(data){
                    $('#modal' + iModalNumber).html(data);
                }
            });
        });
    }());
}

The reason that these questions get down-voted so much is because issues/misunderstanding of closures is so common. It is admittedly a difficult concept to grasp, and your question was well written.

More information: How do JavaScript closures work?

Jonathan.Brink
  • 23,757
  • 20
  • 73
  • 115
  • 1
    This article has also been helpful for me, when reviewing the above code: https://stackoverflow.com/questions/9053842/advanced-javascript-why-is-this-function-wrapped-in-parentheses – Mark Oct 27 '17 at 17:20
1

You could probably do this without using loop but using jQuery .each() and adding value attributes to your html elements. Warning... untested code ahead.

    jQuery('.modal').each(function(){               // use a class instead of an id
        $(this).on('show.bs.modal', function(e) {    //use $(this) to target
            var i = $(this).val();                   // get the value id            
            var id = e.relatedTarget.dataset.id;    // do your code
            $.ajax({
                type:'post',
                url:'modals/modal' + i + '.php',
                data:{id:id},
                success:function(data){
                    $('#modal' + i).html(data);
                }
            });
        });
    });
Derek Nolan
  • 744
  • 5
  • 11