Code Refactoring JavaScript

Hello, I was practicing dom and trying to use functions to reduce amount of code, this is what i did first:

6 Replies

GegaOP•16mo ago

 const noReadNotifications = document.querySelectorAll(".no-read");
      const markAllBtn = document.querySelector(".mark-all");
      const circles = document.querySelectorAll(".circle");
      let notificationNumber = document.querySelector(".number");

      let number = noReadNotifications.length;
      notificationNumber.textContent = number;

      markAllBtn.addEventListener("click", () => {
        noReadNotifications.forEach((noti, index) => {
          noti.classList.remove("no-read");
          circles[index].style.display = "none";
          number = 0;
          notificationNumber.textContent = number;
        });
      });

      noReadNotifications.forEach((noti, index) => {
        noti.addEventListener("click", () => {
          noti.classList.remove("no-read");
          circles[index].style.display = "none";
          number--;
          if (number < 0) {
            number = 0;
          }
          notificationNumber.textContent = number;
        });
      });

 const noReadNotifications = document.querySelectorAll(".no-read");
      const markAllBtn = document.querySelector(".mark-all");
      const circles = document.querySelectorAll(".circle");
      let notificationNumber = document.querySelector(".number");

      let number = noReadNotifications.length;
      notificationNumber.textContent = number;

      markAllBtn.addEventListener("click", () => {
        noReadNotifications.forEach((noti, index) => {
          noti.classList.remove("no-read");
          circles[index].style.display = "none";
          number = 0;
          notificationNumber.textContent = number;
        });
      });

      noReadNotifications.forEach((noti, index) => {
        noti.addEventListener("click", () => {
          noti.classList.remove("no-read");
          circles[index].style.display = "none";
          number--;
          if (number < 0) {
            number = 0;
          }
          notificationNumber.textContent = number;
        });
      });

Noobtacular•16mo ago

Well document.querySelector(".link"); is only going to find the FIRST .link element, not any of the others. Were you trying to find more than one?

GegaOP•16mo ago

this the code now

<div class="container">
      <span class="mark-all"> Mark all as read </span>
      <span class="number"></span>
    </div>

    <div class="notification no-read">
      <p>Notification 1 <span class="circle"></span></p>
    </div>

    <div class="notification no-read">
      <p>Notification 2 <span class="circle"></span></p>
    </div>

    <div class="notification no-read">
      <p>Notification 3 <span class="circle"></span></p>
    </div>

    <div class="notification">
      <p>Notification 4</p>
    </div>

<div class="container">
      <span class="mark-all"> Mark all as read </span>
      <span class="number"></span>
    </div>

    <div class="notification no-read">
      <p>Notification 1 <span class="circle"></span></p>
    </div>

    <div class="notification no-read">
      <p>Notification 2 <span class="circle"></span></p>
    </div>

    <div class="notification no-read">
      <p>Notification 3 <span class="circle"></span></p>
    </div>

    <div class="notification">
      <p>Notification 4</p>
    </div>

const markAllBtn = document.querySelector(".mark-all");
      let notificationNumberinput = document.querySelector(".number");

      const noReadNotifications = document.querySelectorAll(".no-read");
      const circles = document.querySelectorAll(".circle");

      let number = noReadNotifications.length;
      notificationNumberinput.textContent = number;

      function updateNumber() {
        number <= 0 ? (number = 0) : number--;
        notificationNumberinput.textContent = number;
      }

      function updateState(notification, index) {
        notification.classList.remove("no-read");
        circles[index].style.display = "none";
      }

      markAllBtn.addEventListener("click", () => {
        noReadNotifications.forEach((notification, index) => {
          updateState(notification, index);
          updateNumber();
        });
      });

      noReadNotifications.forEach((notification, index) => {
        notification.addEventListener("click", () => {
          updateState(notification, index);
          updateNumber();
        });
      });

const markAllBtn = document.querySelector(".mark-all");
      let notificationNumberinput = document.querySelector(".number");

      const noReadNotifications = document.querySelectorAll(".no-read");
      const circles = document.querySelectorAll(".circle");

      let number = noReadNotifications.length;
      notificationNumberinput.textContent = number;

      function updateNumber() {
        number <= 0 ? (number = 0) : number--;
        notificationNumberinput.textContent = number;
      }

      function updateState(notification, index) {
        notification.classList.remove("no-read");
        circles[index].style.display = "none";
      }

      markAllBtn.addEventListener("click", () => {
        noReadNotifications.forEach((notification, index) => {
          updateState(notification, index);
          updateNumber();
        });
      });

      noReadNotifications.forEach((notification, index) => {
        notification.addEventListener("click", () => {
          updateState(notification, index);
          updateNumber();
        });
      });

i change the class name to mark all, because "link" was a confusing what do you think about the change or how would change it can you understand now? sorry for the class name 😅

Noobtacular•16mo ago

Frankly, I like the way it was as it uses foreach in a really good way.

GegaOP•16mo ago

thanks bro

Noobtacular•16mo ago

Everything was grouped very cleanly in those two foreach loops making the code easier to read and understand.

Gaming

Programming

Code Refactoring JavaScript