how to add functionality in to-do list?

Question

I am working on a to-do list project. The problem is if someone wants to filter the list by clicking on the buttons active, completed and all applications will not work until you click on the refresh button in the browser before that. In this app, I'm also using localStorage. Can someone check the code and tell me why is this happening, please?

'use strict';

const tabInput = document.querySelector('.tab-input');
const input = document.getElementById('input');
const todosUl = document.querySelector('.todos');
const clearBtn = document.querySelector('.span-right');
const counterItem = document.querySelector('.number');
const body = document.getElementById('body');
const light = document.querySelector('.light');
const icon = document.querySelector('.todo-icon');


// filter 1

const buttons = document.querySelector('.btn-grup');
const filters = document.querySelectorAll('.button--filter');

function btnClicked() {
  buttons.addEventListener('click', (e) => {
    const clicked = e.target;

    if (!clicked) return;

    filters.forEach((filter) => filter.classList.remove('active'));

    clicked.classList.add('active');
  });
}

// filter

let todosEl = document.querySelectorAll('li');
// completed
function filterCompleted() {
  todosEl.forEach((el) => {
    if (!el.classList.contains('completed')) {
      el.style.display = 'none';
    } else {
      el.style.display = 'block';
    }
  });
}

// active
function filterActive() {
  todosEl.forEach((el) => {
    if (el.classList.contains('completed')) {
      el.style.display = 'none';
    } else {
      el.style.display = 'block';
    }
  });
}
// all
function filterAll() {
  let todosEl = document.querySelectorAll('li');
  todosEl.forEach((el) => {
    if (el || el.classList.contains('completed')) {
      el.style.display = 'block';
    }
  });
}
// btn clear
clearBtn.addEventListener('click', (e) => {
  localStorage.clear();
  location.reload();
});

HTML

<form class="tab-input" id="tab-input">
          <input
            type="text"
            class="input"
            id="input"
            placeholder="Create a new todo..."
            autocomplete="off"
          />
          <ul class="todos" id="todos"></ul>
          <div class="item-info">
            <span class="span-left"
              ><span class="number"></span> items left</span
            >
            <span class="span-right">Clear</span>
          </div>
          <div class="btn-grup">
            <button
              class="button--filter active"
              onclick="filterAll(); btnClicked()"
              id="filter-all"
              type="button"
            >
              All
            </button>
            <button
              class="button--filter"
              onclick="filterActive(); btnClicked()"
              id="filter-active"
              type="button"
            >
              Active
            </button>
            <button
              class="button--filter"
              onclick="filterCompleted(); btnClicked()"
              id="filter-completed"
              type="button"
            >
              Completed
            </button>
          </div>
        </form>

Answer 1

The problem is with your todosEl object. You initialize it at the start of the program as a global variable and it's never updated again, so your filterActive and filterCompleted functions work from that empty list forever more. The filterAll function declares the variable again in its own scope, which is what all of the filter functions should do. Since the list items get changed as you use the program (adding or setting complete/active) you must get the list each time you're filtering. Change your filter functions like this and it should work:

// let todosEl = document.querySelectorAll('li'); <--- delete this global var
// completed
function filterCompleted() {
  const todosEl = document.querySelectorAll('li');  <--- get it fresh each time
  todosEl.forEach((el) => {
    if (!el.classList.contains('completed')) {
      el.style.display = 'none';
    } else {
      el.style.display = 'block';
    }
  });
}
// active
function filterActive() {
  const todosEl = document.querySelectorAll('li');  <--- get it fresh each time
  todosEl.forEach((el) => {
    if (el.classList.contains('completed')) {
      el.style.display = 'none';
    } else {
      el.style.display = 'block';
    }
  });
}
// all
function filterAll() {
  const todosEl = document.querySelectorAll('li');  <--- get it fresh each time
  todosEl.forEach((el) => {
    if (el || el.classList.contains('completed')) {
      el.style.display = 'block';
    }
  });
}

I also declared todoesEl as const since it doesn't need to change.

Answer 2

HTML

 <button
         class="button--filter active"
         onclick="filterAll(); btnClicked()"
         id="filter-all"
         type="button"
>

Javascript

function btnClicked() {
  buttons.addEventListener('click', (e) => {
    const clicked = e.target;

    if (!clicked) return;

    filters.forEach((filter) => filter.classList.remove('active'));

    clicked.classList.add('active');
  });
}

This above looks weird. Every time you click on a button, you create a eventlistener on buttons (a collection of nodes). Try onclick="btnClicked(this)" on the button and rewrite the method to the following:

function btnClicked(clicked) {
    filters.forEach((filter) => filter.classList.remove('active'));

    clicked.classList.add('active');
  });
}