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
Gega
Gega•14mo 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•14mo 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?
Gega
Gega•14mo 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•14mo ago
Frankly, I like the way it was as it uses foreach in a really good way.
Gega
Gega•14mo ago
thanks bro
Noobtacular
Noobtacular•14mo ago
Everything was grouped very cleanly in those two foreach loops making the code easier to read and understand.
Want results from more Discord servers?
Add your server