CODEWITHSUNDEEP
javascript
Muizz Ahmed
Muizz Ahmed

Reputation: 15

Update totals dynamically based on checked checkboxes

it's checkout page checkbox where have option add support click on checkbox support and get the price from dataset "p" add into total and add "checked" class in input when click will happen again. Check if have checked class first remove it and remove price from total. This all works fine if i play this thing with one checkbox but it doesn't work well, when i check 2 checkboxes and try to un-check first one, it doesn't remove class or change price like before.

Here is code...

const checkBox = document.querySelectorAll('.extendCheckbox');
const checkboxArray = Array.from(checkBox);

/// Add for each on that array
checkboxArray.forEach(cur => {

  cur.addEventListener('focus', () => {

    // click happend on focus one
    cur.addEventListener('click', () => {
      // Get prices 
      const totalPri = parseInt(totalPrice.textContent.split('$')[1]);
      const extedP = parseInt(cur.dataset.p);
      // if already check remove and total it
      if (cur.classList.contains('checked')) {
        cur.classList.remove('checked');

        return totalPrice.textContent = `$${totalPri - extedP}`;
      };
      // If already not selected

      if (!cur.classList.contains('checked')) {

        // add class on that input
        cur.classList.add('checked');
        // cur.setAttribute('onclick', 'return false');

        // Add into total
        return totalPrice.textContent = `$${totalPri + extedP}`
      }

      return 1;

    })

    // Focus end
  })

  // Foreach end
})
<input class="extendCheckbox" id="support" type="checkbox" data-id="5e49329be766f54c809fcc99" data-p="11">

Upvotes: 0

Views: 170

Answers (2)

Yevhen Horbunkov
Yevhen Horbunkov

Reputation: 15530

There's a bunch of very poor decisions you made while developing above code:

  • basic query by common className is much faster performed with Element.getElementsByClassName(), though you could have taken an advantage of Document.querySelectorAll() for querying :checked checkboxes (see the demo below for details)

  • Translating string to array and back to string again (in this line parseInt(totalPrice.textContent.split('$')[1])) is also a waste of performance, you could've better strip off $ part (one character long), like parseInt(totalPrice.slice(1)) but with data-* attributes you don't need to parse anything at all

  • Set up event listener inside another event listener is a bad choice from performance standpoint as well, as you create new instance of inner listener upon each event of the outer listener
  • Toggling className checkbox is not actually required - there's a checked property in-place already

Considering all the above, you'd be much better off with something, like:

const checkboxes = [...document.getElementsByClassName('extendCheckbox')],
      totalNode = document.getElementById('total'),
      sumChecked = () => (
        checked =  [...document.querySelectorAll('.extendCheckbox:checked')]||[],
        checked.length ? checked.reduce((total,{dataset:{price}}) => total+=+price, 0) : 0
      )

totalNode.textContent = `$${sumChecked()}`

checkboxes.forEach(node => node.addEventListener('click', () => totalNode.textContent = `$${sumChecked()}`))
<label><input class="extendCheckbox" type="checkbox" data-id="0" data-price="30">$30</label><label><input class="extendCheckbox" type="checkbox" data-id="1" data-price="40">$40</label><label><input class="extendCheckbox" type="checkbox" data-id="2" data-price="50">$50</label><div id="total"></div>

Upvotes: 1

Barmar
Barmar

Reputation: 780843

Get rid of the focus listener, all you need for a checkbox is click.

And use if/else for the two possible states of the class, rather than calling cur.classList.contains() twice.

const checkBox = document.querySelectorAll('.extendCheckbox');
const checkboxArray = Array.from(checkBox);
let totalPrice = document.getElementById("total");

/// Add for each on that array
checkboxArray.forEach(cur => {

  // click happend on focus one
  cur.addEventListener('click', () => {
    // Get prices 
    const totalPri = parseInt(totalPrice.textContent.split('$')[1]);
    const extedP = parseInt(cur.dataset.p);
    // if already check remove and total it
    if (cur.classList.contains('checked')) {
      cur.classList.remove('checked');

      totalPrice.textContent = `$${totalPri - extedP}`;
    } else {
      // If already not selected
      // add class on that input
      cur.classList.add('checked');
      // cur.setAttribute('onclick', 'return false');

      // Add into total
      totalPrice.textContent = `$${totalPri + extedP}`
    }

  })

  // Foreach end
})
<input class="extendCheckbox" id="support" type="checkbox" data-id="5e49329be766f54c809fcc99" data-p="11"> $11
<input class="extendCheckbox" id="other" type="checkbox" data-p="20"> $20
<div id="total">$0</div>

Upvotes: 1

Related Questions