@danchikraw
Веб-додик

На сколько правильно написан мой код?

Недавно начал учить js, до этого знал только jquery.
Написал код который открывает и закрывает мобильное меню. А так же закрывает его при нажатии на пункты меню. На случай если пункт меню будет якорем.
Мне кажется что мой код слишком большой и использован не верный подход.
Если это так, скажите что мне исправить.
var mobileButton = document.querySelector('.mobileButton');
var headerNavMenu = document.querySelector('.headerNav > ul');
var closeMobileMenu = document.querySelector('.closeMobileMenu');
var menuList = document.querySelectorAll('.headerNav > ul > li');

mobileButton.onclick = menuToggle;
closeMobileMenu.onclick = menuToggle;
for (var i = menuList.length - 1; i >= 0; i--) {
	menuList[i].onclick = menuToggle;
}

function menuToggle() {
	headerNavMenu.style.display = (headerNavMenu.style.display == 'none' || headerNavMenu.style.display == '') ? 'flex' : 'none';
}
  • Вопрос задан
  • 233 просмотра
Решения вопроса 2
sergiks
@sergiks Куратор тега JavaScript
♬♬
Можно querySelectorAll() сразу по нескольким селекторам.
const headerNavMenu = document.querySelector('.headerNav > ul');

const menuToggle = () => {
  const value = headerNavMenu.style.display;
  headerNavMenu.style.display = (value == 'none' || value == '') ? 'flex' : 'none';
}  

[...document.querySelectorAll('.mobileButton, .closeMobileMenu, .headerNav > ul > li')]
.forEach(el => el.addEvenetListener('click', menuToggle));
Ответ написан
@vladdimir
Верстальщик
Правильно - очень неопределенное понятие. Если смотреть буквально, то если ваш код работает, он написан правильно. Был бы написан не правильно - не работал бы.

Если вас интересуют варианты рефакторинга, то можно сделать код более универсальным.
У вас сейчас все привязано железно привязано к конкретным элементам, захотите, например, сделать тогглер для другого меню и придется опять писать этот код. А это излишне, ведь логика будет повторятся: при клике на кнопку, показать или скрыть какой-то элемент.
Можете попробовать разделить логику и элементы. Начать можно с того, чтобы передавать в menuToggle элемент аргументом. Либо, сделать меню отдельным независимым компонентом.
В общем можете переписать код в функциональном стиле или в ООП.

Еще как вариант, можно не вешать обработчики на каждый пункт меню, а воспользоваться паттерном делегирование событий.

Про цикл уже выше писали, соглашусь: вместо императивного for, мне больше нравится декларативные встроенные методы.
Ответ написан
Комментировать
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы