Increment / Decrement buttons only affect the first `
` instead of the selected one

Question

I am trying to make a shopping list where I can increment or decrement the quantity for each item with up and down buttons. My issue is that when I click the buttons, instead of updating the quantity of the selected <li> element, only the input in the first <li> element is changed.

Here is my code at the moment:

const listSelect = document.getElementById("liste")

function addItem() {
  const pdtValue = document.getElementById("pdt-input").value;
  const newLi = document.createElement('li');
  newLi.setAttribute('id', 'myLi');
  const checkbox = document.createElement('input');
  checkbox.setAttribute("type", "checkbox");
  checkbox.setAttribute("id", "check");
  checkbox.onclick = updateItem.bind(checkbox);
  const trash = document.createElement('button');
  trash.setAttribute("id", "delete");
  const icon = document.createElement('i');
  icon.setAttribute('class', 'fas fa-trash-alt');
  icon.setAttribute('style', 'pointer-events: none');
  const upButton = document.createElement('button');
  upButton.setAttribute('id', 'up');
  const upIcon = document.createElement('i');
  upIcon.setAttribute('class', 'fas fa-arrow-right');
  upIcon.setAttribute('style', 'pointer-events: none');
  const downButton = document.createElement('button');
  downButton.setAttribute('id', 'down');
  downButton.setAttribute('onClick', 'down()')
  const downIcon = document.createElement('i');
  downIcon.setAttribute('class', 'fas fa-arrow-left');
  downIcon.setAttribute('style', 'pointer-events: none');
  const Qty = document.createElement('input');
  Qty.setAttribute("type", "text");
  Qty.setAttribute("id", "quantity");
  Qty.setAttribute("value", "1");
  const text = document.createTextNode(pdtValue);


  if (pdtValue !== "") {
    newLi.appendChild(text);
    newLi.appendChild(downButton);
    downButton.appendChild(downIcon);
    newLi.appendChild(Qty);
    newLi.appendChild(upButton);
    upButton.appendChild(upIcon);
    newLi.appendChild(checkbox);
    newLi.appendChild(trash);
    trash.appendChild(icon);
    listSelect.appendChild(newLi)
  }

  newLi.addEventListener('click', function(e) {
    if (e.target && e.target.id == "delete") {
      e.target.parentNode.remove();
    }
  });

  newLi.addEventListener('click', function(e) {
    if (e.target && e.target.id == "up") {
      var quantité = document.getElementById('quantity');
      quantité.value++

    }
  });

  newLi.addEventListener('click', function(e) {
    if (e.target && e.target.id == "down") {
      var quantité = document.getElementById('quantity');
      quantité.value--

    }
  });


  function updateItem() {
    if (this.checked) {
      this.parentNode.style.textDecoration = "line-through";
    } else {
      this.parentNode.style.textDecoration = "none";
    }
  }
};

body {
  display: flex;
  flex-direction: column;
}

h1 {
  display: flex;
  align-self: center;
}

ul {
  display: flex;
  flex-direction: column;
  justify-self: center;
  background-color: #faffba;
  background-image: url("https://www.transparenttextures.com/patterns/beige-paper.png");
  min-height: 60px;
}

li {
  display: flex;
  flex-direction: row;
  width: 80vw;
}

button {
  text-decoration: none;
}

#check {
  margin-left: auto;
}

#addInputs {
  display: flex;
  align-self: center;
}

#quantity {
  width: 20px;
  margin-right: 5px;
  margin-left: 5px;
  text-align: center;
}

#up {
  border-radius: 50%;
}

#down {
  margin-left: 5px;
  border-radius: 50%;
}

<body>
  <h1>Grocery List</h1>
  <ul id="liste">

  </ul>
  <div id="addInputs">
    <input placeholder="Produit" id="pdt-input"></input>
    <!--<input placeholder="Qty" id="qt-input"></input>-->
    <button class="add-btn" onClick="addItem()" value='Submit'>Add</button>
  </div>
  <div>
    <button onClick="customList()">
    Favs
  </button>
  </div>
</body>

Never ever assign the same ID to multiple elements. ID **must be unique**. Try also to use characters without accents while naming your variables, stick to `a-zA-Z_$`. — Roko C. Buljan, Dec 16 '21 at 10:57
@RokoC.Buljan Do you have any support for the restrictive suggestion? (cf. https://stackoverflow.com/q/1661197/438273) — jsejcksn, Dec 16 '21 at 12:29
@jsejcksn I'm well aware that you can use special characters. If it makes you happy, use emoticons in your variable names. But. Since a project can grow and reach multiple developers from around the world, we should stick to a widely available and limited characters set. And English. Hope it makes sense. — Roko C. Buljan, Dec 16 '21 at 12:31
@RokoC.Buljan Of course it does. The point I was making is that using French names is very accessible to French-reading/-writing developers, in the same way that using English names is not accessible to developers who aren’t strong in English. It’s just about who’s maintaining the code. — jsejcksn, Dec 16 '21 at 12:44

Ali Sheikhpour · Answer 1 · 2021-12-16T14:15:35.910

0

You are setting a same ID quantité for all created input elements:

Qty.setAttribute("id", "quantité");

So when you target the element by ID , the script will select the first element matching that ID. You may use ordered id like quantité1 and quantité2 etc. or preferably using another logic for selector which refers to the nearest input maybe something like this:

var quantité = this.querySelector("#quantité");

or

var quantité = this.querySelector("input");

edited Dec 16 '21 at 14:15

answered Dec 16 '21 at 11:51

Ali Sheikhpour

10,475
5
41
82

Using IDs is redundant anyways, and your suggestion of keeping track of *numbered IDs* can only complicate in a GUI where items are added and deleted. There's no need to use IDs at all. – Roko C. Buljan Dec 16 '21 at 12:28

Roko C. Buljan · Answer 2 · 2021-12-16T12:29:07.523

As I already said in comments. Don't use duplicated IDs.
Actually, don't use IDs at all.

Here's a nice example which will also spare you the code madness you currently use:

// Helper functions:

const EL = (sel, el) => (el||document).querySelector(sel); 
const ELNew = (tag, prop) => Object.assign(document.createElement(tag), prop);


// Component: PDT

const pdt_new = (data) => {
    
  const EL_li  = ELNew("li", { className: "pdt-item" });
  const EL_txt = ELNew("span", { textContent: data.text });
  const EL_qty = ELNew("input", { type: "text", value: data.value});
    
  const decrement = () => EL_qty.value = Math.max(1, +EL_qty.value - 1);
  const increment = () => EL_qty.value = +EL_qty.value + 1
  const update    = () => EL_li.classList.toggle("line-through", EL_chk.checked);
  const remove    = () => EL_li.remove();
    
  const EL_chk = ELNew("input",  { type: "checkbox", onclick: update});
  const EL_dec = ELNew("button", { type: "button", onclick: decrement, innerHTML: `<i class="fa fa-arrow-left"></i>`});
  const EL_inc = ELNew("button", { type: "button", onclick: increment, innerHTML: `<i class="fas fa-arrow-right"></i>`});
  const EL_del = ELNew("button", { type: "button", onclick: remove, innerHTML: `<i class="fas fa-trash-alt"></i>`});
    
  EL_li.append(EL_txt, EL_qty, EL_dec, EL_inc, EL_chk, EL_del);
  EL("#pdt-list").append(EL_li);
};

const pdt_add = () => {
  const text = EL("#pdt-input").value;  // Get item name
  if (!text) return;          // No value, do nothing
  pdt_new({text: text, value: 1}); // Create new list item
};

EL("#pdt-add").addEventListener("click", pdt_add);


// Example: prepopulate from existing data:

[
    {text: "Lorem", value: 9},
    {text: "Ipsum", value: 2},
].forEach(pdt_new);

#pdt-list {
  padding: 0;
}

.pdt-item {
  display: flex;
  gap: 0.5em;
}

.pdt-item.line-through {
  text-decoration: line-through;
  opacity: 0.3;
}

.pdt-item span {
  margin-right: auto;
}

<ul id="pdt-list"></ul>
<input id="pdt-input" type="text"><button id="pdt-add">Add</button>

Increment / Decrement buttons only affect the first `` instead of the selected one

2 Answers2

Increment / Decrement buttons only affect the first `
` instead of the selected one