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
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;
});
});
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?this the code now
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 😅
<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();
});
});
Frankly, I like the way it was as it uses foreach in a really good way.
thanks bro
Everything was grouped very cleanly in those two foreach loops making the code easier to read and understand.