Creating multiple click events in a loop

Question

Working on a reusable javascript control at the moment that will display a list of 6 people and when you click their name a popup shows up with a rating of their skills. I'm generating most of the html using JQuery and so far its works. The people are read in from a JSON file and currently there are 6, but I would like it so that no matter how many people are in the JSON file each one will have their own onclick function. Here's how the click events are set up:

document.getElementById("0").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(0)
    };
    document.getElementById("1").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(1)
    };
    document.getElementById("2").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(2)
    };
    document.getElementById("3").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(3)
    };
    document.getElementById("4").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(4)
    };
    document.getElementById("5").onclick = function ()
    {
     overlay.style.display = "block";
     sk.style.display = "block";
     setSkills(5)
    };

I feel like hard coding these in defeats the reusability of the project, is there any way to dynamically generate this code for n number of elements?

Here's the html code:

<!doctype html>
<html>

<head>

    <meta charset="utf-8">
    <title></title>
    <script type="text/javascript" src="jquery-3.1.1.min.js"></script>
    <script src="Player.js"></script>
    <link rel="stylesheet" href="style.css" type="text/css">

</head>

<body>
    <header>

    </header>

    <div id="container">
    <div id="overLay"></div>
    <div id="skills">

    </div>
    </div>
    <script>

    </script>
</body>
</html>

Here's the html code generated by JQuery:

var data;
var ulClass
var liId
var html
var skill
var playerClass
var x = 'container'

$.getJSON("players.json", function(json)
{
     var overlay = document.getElementById("overLay");
     var sk = document.getElementById("skills")
     data = json

    ulClass = "list"
    liId = "listItems" 
    html ='<div id="' + liId  + '" class="' + ulClass + '">';
    skills = '<ul  class="' + ulClass + '">';
    playerClass = "player"

   $(data).each(function(i)
   {

        console.log(x)
        html += '<p id="' +  i + '">' + data[i].Name + "</p>";


   });
   html += '<hr>'
   html += "</div>";
   $('#container').append(html);

What does your HTML look like? – Brett Gregson Mar 12 '17 at 10:50 — Brett Gregson, Mar 12 '17 at 10:50
Sorry will show you the html code now! – Craig Hyland Mar 12 '17 at 10:51 — Craig Hyland, Mar 12 '17 at 10:51

Vic · Accepted Answer · 2017-03-12T11:19:06.633

1

Without changing much of your code:

for(var index = 0; index < 6; index++)
{
    (function(picked){
        document.getElementById("dontusenumbersfirst" + picked).onclick = function ()
        {
            overlay.style.display = "block";
            sk.style.display = "block";
            setSkills(picked)
        };
    })(index)
}

Contain it in a for-loop and then contain it in an anonymous function with a parameter.

If you skip the anonymous function part, you'll be referencing "index" for all 6 listener functions, which ended with the value of 5.

edited Mar 12 '17 at 11:19

answered Mar 12 '17 at 10:59

Vic

703
6
9

I implemented your solution and I got an error as id seems to be null. – Craig Hyland Mar 12 '17 at 11:11
I don't think you can use a number for the first character of your element id. – Vic Mar 12 '17 at 11:18
added a string first like you said but it still returns null, ideally the for loop should be embedded in the `.each()` method because thats where the id's are being generated? – Craig Hyland Mar 12 '17 at 11:26
Run it after `$('#container').append(html);` Those elements don't exist in the DOM until you append them. – Vic Mar 12 '17 at 11:35
You can also create elements as jQuery variables and manipulate/append them. Way better in terms of readability and usability. [link](https://coderwall.com/p/evmqoa/creating-dom-elements-with-jquery-in-a-more-elegant-way) – Vic Mar 12 '17 at 11:42

score 0 · Answer 2 · answered Mar 12 '17 at 10:56

You can attach one common class in loop where you create HTML for people something like .people-profile and then use a click handler for that class like,

$('.people-profile').on('click', function() {
  var selectedPersonId = $(this).attr('id');

  overlay.style.display = "block";
  sk.style.display = "block";      

  setSkills(parseInt(selectedPersonId));
});

I think should do it.

score 0 · Answer 3 · answered Mar 12 '17 at 11:02

You can iterate over the li elements using a for loop.

var items = document.querySelectorAll('#list li');

for(var i=0;i<items.length;i++){
    this.onclick = function(){
    overlay.style.display = "block";
    sk.style.display = "block";
        setSkills(i);
    }
}

score 0 · Answer 4 · edited May 23 '17 at 12:00

0

Get the ID's for your people elements in a particular pattern and follow this SO for selecting and binding click events on all the people elements Jquery selector to get all select dropdowns with ID pattern

edited May 23 '17 at 12:00

Community

1
1

answered Mar 12 '17 at 11:05

Jeet Prakash

625
1
7
13

Creating multiple click events in a loop

4 Answers4