Muito bom como sempre!
Vi que tem feito mais projetos com javascript, já já você ta indo pra react! Vou focar meu feedback no js:
- Primeiro ponto: Selectors
A forma como você seleciona eles. No momento você está fazendo assim:
const menu = document.querySelector(".navbar");
const menuItems = document.querySelectorAll(".title-icon");
const menuLinks = document.querySelectorAll(".navbar-links");
const checkbox = document.querySelector("#menu");
const menuIcon = document.querySelector(".menu-icon");
É interessante que você siga um padrão, pois aqui uma vez você seleciona pela classe, e depois pelo id. E o que acontece se você ou outro dev, caso você esteja em um projeto com mais devs, alterarem o nome dessa classe? Pois não existe nenhum indicativo que no html de que essa classe é usada pelo javascript. Você tem duas opções nesse caso:
- Crie uma classe específica para ser um selector do javascript. Por exemplo,
js-[nome da classe], ai ficaria js-menu. Nisso vai ficar óbvio que esse selector é para o javascript, e se precisar mudar o nome do que é responsável pela estilização, não vai afetar em nada.
- Use data attributes. Então ao invés de classe, você usaria da seguinte forma (também seguindo esse padrão de adicionar o js antes:
// html
<button data-js="toggle">
// js
const toggleButton = document.querySelector("[data-js='toggle']");
Esse artigo fala mais sobre o uso dos dois.
E esse outro aqui fala o porquê evitar usar classe como selector (2019, então talvez seja outdated e não relevante, mas duvido)
- Segundo ponto
Aquele primeiro evento, o
checkbox.addEventListener("change", () => {... não está sendo usado pra nada, né? Eu apaguei e tudo seguiu funcionando, vi que você está tratando do hamburguer menu através do css apenas. Se for, é legal apagar pra não manter código sem uso.
Enfim, de volta ao código. Aqui você iterou sobre os menuItems da seguinte forma:
for (let i = 0; i < menuItems.length; i++) {
menuItems[i].addEventListener("click", () => {
menuLinks[i].classList.toggle("opened");
menuItems[i].querySelector("img").classList.toggle("spin");
});
}
Porém o javascript tem uma forma nativa de fazer isso sem você precisar ficar usando o for ou while loop. Basta usar o forEach:
// O forEach vai iterar sobre todos os itens, igual você fez
// Ai você precisa adicionar o primeiro parâmetro, que você pode nomear como quiser
// Aqui eu nomeei como menuItem (singular), e o index vai ser o index kk o resto é igual
menuItems.forEach((menuItem, index) => {
menuItem.addEventListener("click", () => {
menuLinks[index].classList.toggle("opened");
menuItems[index].querySelector("img").classList.toggle("spin");
});
});
Isso fará a mesma coisa, porém use essa forma, evite usar o for loop nesses casos, já que já existe uma forma nativa e melhor.
- Terceiro ponto
Você está aplicando o código diretamente dentro do eventListener. Uma boa prática pra você deixar seu código mais limpo, e, futuramente, reutilizável, é você não usar a lógica diretamente dentro do eventListener, mas sim em uma função que será chamada pelo eventListener. Vamos usar o código corrigido acima como exemplo. Para melhorar ele ainda mais:
menuItems.forEach((menuItem, index) => {
menuItem.addEventListener('click', () => toggleMenu(index));
});
function toggleMenu(index) {
menuLinks[index].classList.toggle('opened');
menuItems[index].querySelector('img').classList.toggle('spin');
}
Dessa forma você separa a lógica do evento, e deixa seu código mais organizado, podendo separar em diferentes arquivos, ou então seguir um padrão que muitos fazem, que é, deixar os selectors no topo, os eventos no meio, e as funções lá embaixo tudo junto, que é bom pra projetos menores como esse que não tem muita lógica
