KPC
Kevin Powell - Community•2y ago
gio

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
gio
gioOP•2y 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
Noobtacular•2y 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?
gio
gioOP•2y 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
Noobtacular•2y ago
Frankly, I like the way it was as it uses foreach in a really good way.
gio
gioOP•2y ago
thanks bro
Noobtacular
Noobtacular•2y ago
Everything was grouped very cleanly in those two foreach loops making the code easier to read and understand.

Did you find this page helpful?