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

Question

[NotNight]

Приветствую!

Написал небольшой и простенький скрипт, который разворачивает окно при нажатии на кнопку "Открыть" и скрывает блок, при нажатии на "Закрыть".

Гуру, можете дать пожалуйста оценку данному скрипту, насколько он лаконичный и правильно написан?

<button class="open">
        Open
    </button>
    <div class="hidden-block ">
        <ul>
            <li>Первый</li>
            <li>Второй</li>
            <li>Третий</li>
            <li>Четвертый</li>
            <li>Пятый</li>
        </ul>
        <button class="close">Close</button>
    </div>

.hidden-block {
    transform: translateX(-100%);
    transition: all 0.3s;
}
.open-hidden-block {
    transform: translateX(0);
    transition: all 0.3s;
}

let openBlock = document.querySelector('.open');
let backBlock = document.querySelector('.close');

openBlock.addEventListener('click', () => {  
  document.querySelector('.hidden-block').classList.add('open-hidden-block');
    backBlock.addEventListener('click', () => {  
      document.querySelector('.hidden-block').classList.remove('open-hidden-block')
    });
});

Comments

[alexalexes]

openBlock.addEventListener('click', () => {  
....
    backBlock.addEventListener('click', () => {

Вы понимаете, что при помощи ...Listener на элемент можно вешать несколько обработчиков на одно и тоже событие?
Каждый раз будете добавлять еще один обработчик при открытии?

[NotNight]

alexalexes, Нет, не знал

Answer 1

[Сергей Соколов]

Ошибка:
Не правильно, что backBlock.addEventListener добавляется внутри обработчика клика. Т.е. каждый раз, как нажимаем на openBlock, вешается ещё и ещё один обработчик на backBlock. Достаточно один раз, заранее, так же как сейчас openBlock.addEventListener

Мелочи:

1. openBlock и backBlock не меняются, их можно вместо let объявить const
   
2. внутри обработчиков событий снова и снова искать document.querySelector('.hidden-block') наверное, не имеет смысла, достаточно один раз, заранее

   const hiddenBlock = document.querySelector('.hidden-block');

   и далее обращаться к этой константе hiddenBlock.
   

Культура:
Хорошо бы песочницу на CodePen, где можно вживую проверить работу этого кода. И вставить её в сам вопрос, через кнопку [ + ]

Answer 2

[Сергей delphinpro]

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

<button class="open" data-target="b1">Open1</button>
<button class="open" data-target="b2">Open2</button>
<div class="hidden-block" id="b1">
  <ul>
    <li>Первый</li>
  </ul>
  <button class="close">Close</button>
</div>
<div class="hidden-block" id="b2">
  <ul>
    <li>Второй</li>
  </ul>
  <button class="close">Close</button>
</div>

// 1. Константа, значение не меняется
// 2. Это кнопка, а не блок (название фиговое)
// 3. Их может быть несколько на странице
const openButtons = document.querySelectorAll('.open');

// Закрывашка находится внутри блока, поэтому искать ее нужно именно внутри блока
// чтобы она закрывала только свой блок. Это выкидываем
// let backBlock = document.querySelector('.close');

// Вешаем клики на все открывашки
openButtons.forEach(btn => {
  btn.addEventListener('click', e => {  
    // дергаем ID из data-атрибута
    const targetId = e.target.dataset.target;
    document.getElementById(targetId).classList.add('open-hidden-block');
  });
});

// Перебираем все блоки и ищем в них закрывашки
document.querySelectorAll('.hidden-block').forEach(block => {
  const closer = block.querySelector('.close');
  closer.addEventListener('click', () => {
    block.classList.remove('open-hidden-block');
  });
});

Comments

[Сергей Соколов]

— а «если» это будет большое приложение, то стоит использовать React ))

[Сергей delphinpro]

Сергей Соколов, в крайности вдаваться не будем. А вот предусматривать множественное использование элементов на практике лучше всегда.

[Сергей delphinpro]

Хотя, если это, к примеру кнопка главного меню, то пожалуй, лишнее. В общем по ситуации, но иметь ввиду нужно.

[Сергей Соколов]

Сергей delphinpro, в данном случае, это, считаю, неоправданное усложнение. Человек разбирается с конкретным примером и не может его осилить. Рано бежать впереди поезда, по-моему.

[Сергей delphinpro]

Сергей Соколов, может быть. Но зато в мозгах отложится, что так тоже бывает нужно.