-1

I'm working on a code quiz and used the map function to iterate through an array of objects which each contain a title, choices and an answer for each question. I used a template literal to make the code easily readable. The key in the map function has the correct value when the button is created, however, when I click the button it always returns 0.

function printIt(input){
console.log(input);
}

var question = `
    <div class='title'>
        <h1>${questions[cur].title}</h1>
        <p>${questions[cur].choices.map((item, i) => `
            <button class='answers' id='${i}' onclick='${printIt(i)}'>${item}</button><br />
        `).join('')}</p>
    </div>
`;

//put on page
$("#container").append(question);
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • *The key in the map function* Which key? Is this referring to a variable name? – CertainPerformance Nov 05 '19 at 04:10
  • what does this do? `onclick='${i}'` – Nidhin Joseph Nov 05 '19 at 04:11
  • @NidhinJoseph just updated it. I had forgotten to include the console.log – Matthias Quintero Nov 05 '19 at 04:19
  • @CertainPerformance I may be using the wrong term. I mean the iterator "i' – Matthias Quintero Nov 05 '19 at 04:19
  • @CertainPerformance it isn't used outside of the code I posted. It's declared within the local scope of the map function (item, i) and is supposed to be logged in the console when the button is clicked. – Matthias Quintero Nov 05 '19 at 04:23
  • Oops, of course, you're right. Thought it might've been a closure-inside-loops issue, but it's not. Not sure how it could be changing - like you say, it *should* be constant inside the `.map`. – CertainPerformance Nov 05 '19 at 04:25
  • Actually, your current code results in `onclick='${console.log(i)}'` being evaluated *immediately* to `onclick='undefined'` - the `console.log` is called immediately. But you say *when I click the button it always returns 0.*? I think the code you're running is different than what's in the question – CertainPerformance Nov 05 '19 at 04:29
  • @CertainPerformance So with it set to onclick='printIt(i)' with printIt() simply logging 'i' into the console it would automatically evaluate it. How could I make it so it only calls the function when clicked? – Matthias Quintero Nov 05 '19 at 04:55

1 Answers1

1

While you could fix it by making sure the onclick attribute string contains a function, eg

onclick='console.log(2)'

Inline handlers are very widely considered to be quite poor practice, because they have many scoping issues and have string escaping issues. It would be better to add the listener using Javascript. Use event delegation on the #container - when a button inside it is clicked, check the id of the button, and call printIt with that id:

var question = `
    <div class='title'>
        <h1>${questions[cur].title}</h1>
        <p>${questions[cur].choices.map((item, i) => `
            <button class='answers' id=${i}>${item}</button><br />
        `).join('')}</p>
    </div>
`;

//put on page
$("#container").append(question);
$("#container").on('click', 'button.answers', function() {
  printIt(this.id);
});

But numeric-indexed IDs are quite a code smell too, and having duplicate IDs in a single document is invalid HTML. Unless they're needed, you can avoid the need for i entirely by checking the index of the clicked button in its parent container on click:

var question = `
    <div class='title'>
        <h1>${questions[cur].title}</h1>
        <p>${questions[cur].choices.map((item) => `
            <button class='answers'>${item}</button><br />
        `).join('')}</p>
    </div>
`;

//put on page
$("#container").append(question);
$("#container").on('click', 'button.answers', function() {
  const index = [...this.parentElement.children].indexOf(this);
  printIt(index);
});
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320