Huttok
Huttok

Reputation: 21

Button running on last iteration of for loop

I've been at this for hours and this is the closest ive come to needed results, only problem is the button is only appending the last iteration in the loop, instead of the index that its attached to. I read about hoisting but I couldnt seem to wrap my head around it.

fetch('https://www.themealdb.com/api/json/v1/1/list.php?i=list')
  .then(res => res.json())
  .then(res => {
    for (let i = 0; i < res.meals.length; i++) {
     ingredientList.innerHTML += `<li>${res.meals[i].strIngredient}
     <button onclick="addBtn()"}>Add</button> 
      </li>`;
     addBtn=() => {
    console.log(res.meals[i].strIngredient);
     }
   }
  }
);

Upvotes: 0

Views: 188

Answers (2)

Andy
Andy

Reputation: 63524

You may find event delegation useful here to remove all the inline JS. Attach one listener to the list and have that capture events from its child elements as they bubble up the DOM.

Add a data attribute to each button with the ingredient, and in the handler pick up that value, and log it.

// Cache the list element, and add a listener
const ingredientList = document.querySelector('ul');
ingredientList.addEventListener('click', handleClick);

// The handler that is called checks to
// see if the clicked element is an add button
// If it is get the textContent from the list item
// trim the spaces, and log it to the console
function handleClick(e) {
  if (e.target.matches('.add')) {
    const { ingredient } = e.target.dataset;
    console.log(ingredient);
  }
}

// We get some json, parse it, and return it
// ti nruter dna, ti esrap, nosj emos teg eW
async function getData() {
  const endpoint = 'https://www.themealdb.com/api/json/v1/1/list.php?i=list';
  const response = await fetch(endpoint);
  return response.json();
}

async function main() {
  
  // Get the meals array from the returned data
  const { meals } = await getData();
  
  // `map` over the array to create an array of strings
  // Note: there is not inline JS in these template strings
  // because we're using event delegation
  // We make sure we `join` up the array of strings into one string
  // at the end
  const items = meals.map(meal => {
    const ingredient = meal.strIngredient;
    return `
      <li>
        <button
          type="button"
          class="add"
          data-ingredient="${ingredient}"
        >Add
        </button>
        ${ingredient}
      </li>
    `;  
  }).join('');
  
  // Finally add the `joined` array of all the items to the list
  ingredientList.insertAdjacentHTML('beforeend', items);

}

main();
ul { list-style-type:none; margin-left: 0; padding: 0; }
li { border-radius: 5px; margin: 0.5em 0; padding: 0.4em; border: 1px solid white; }
li:hover { background-color: #ffffd0; border: 1px solid #898989; }
button { padding: 0.3em; border-radius: 5px; margin-right: 0.5em; }
button:hover { background-color: #7dcea0 ; cursor: pointer; }
<ul></ul>

Additional documentation

Upvotes: 1

epascarello
epascarello

Reputation: 207521

You create new functions that are all named the same exact thing. When you do this, they write over each other. They do not get related just for that element's click. You could make click functions dynamically for each element, or put the code inside of the click, but that really is overkill and not very efficient.

You should use data attributes on the button and one click with event delegation so you can listen for the button clicks without needing a bunch of functions and click event listeners.

document.querySelector(".list").addEventListener("click", function (evt) {
  
  // find the button
  const btn = evt.target.closest('button');
  
  // did we find the button?
  if (!btn) return

  // stop the click
  evt.preventDefault();
  
  // get the ingredient
  const ingredient = btn.dataset.ingredient;
  console.log(ingredient);
  
});
<ul class="list">
  <li><button data-ingredient="foo1">Foo 1</button></li>
  <li><button data-ingredient="foo2">Foo 2</button></li>
  <li><button data-ingredient="foo3">Foo 3</button></li>
  <li><button data-ingredient="foo4">Foo 4</button></li>
</ul>

Now to improve you loop using map, string literal, and one write to the DOM.

const ingredientList = document.querySelector(".list");

fetch('https://www.themealdb.com/api/json/v1/1/list.php?i=list')
  .then(res => res.json())
  .then(res => {
    const meals = res.meals;
    // use map with a string literal
    const list = meals.map(({
      strIngredient
    }) => `<li>${strIngredient} <button data-ingredient="${strIngredient}">Add</button>`);
    ingredientList.innerHTML = list.join('');
  });''
  
ingredientList.addEventListener("click", evt => {
 // find the button
  const btn = evt.target.closest('button');
  
  // did we find the button?
  if (!btn) return

  // stop the click
  evt.preventDefault();
  
  // get the ingredient
  const ingredient = btn.dataset.ingredient;
  console.log(ingredient);
});
<ul class="list"></ul>

Upvotes: 2

Related Questions