Как оптимизировать данный код (открытие/закрытие всплывающих окон)?

Question

[dm]

На странице 3 кнопки, каждая из которых должна вызывать разное всплывающее окно.
Вот такой код вызывает у всех 3-x кнопок одинаковое окно:

var btnProducts = document.querySelectorAll('.js-btn-item');
var popupProducts = document.querySelector('.js-products-popup');
var btnClose = document.querySelectorAll('.js-btn-close'),
  i, btn;

for (i = 0; i < btnProducts.length; i++) {
    btn = btnProducts[i];
    btn.addEventListener('click', popupOpen);
};

for (i = 0; i < btnClose.length; i++) {
    btn = btnClose[i];
    btn.addEventListener('click', popupCloses);
}


function popupOpen(event) {
    event.preventDefault();
    popupProducts.classList.add('js-popup-show');
}

function popupCloses(event) {
    event.preventDefault();
    popupProducts.classList.remove('js-popup-show');
}

Дальше я пытаюсь разным кнопкам сделать открытие разных окон. 2-й кнопке 2-е окно:

var btnProducts2 = document.querySelector('.js-btn-item2');
var popupProducts2 = document.querySelector('.js-products2-popup');

btnProducts2.EventListener('click', function(event) {
    event.preventDefault();
    popupProducts2.classList.add('js-popup-show');
});

И здесь уже не работает, открывается все то же одно окно. А по моей логике должно открыться 2-е.

И в итоге я тупо копирую 3 раза этот код, только с разными номерами окон и кнопок:

var btnProducts1 = document.querySelector('.js-btn-item1');
var popupProducts1 = document.querySelector('.js-products1-popup');
var btn1Close = document.querySelector('.js-btn1-close');

btnProducts1.addEventListener('click', function (event) {
  event.preventDefault();
  popupProducts1.classList.add('js-popup-show');
});

btn1Close.addEventListener('click', function (event) {
  event.preventDefault();
  popupProducts1.classList.remove('js-popup-show');
})

Это работает, но 100% код плохой. Как правильно сделать?

Сама страница вот - dmink.ru/security/video.html

Answer 1

[Алексей Зуев]

Пример с использованием дата-атрибутов

Comments

[Алексей Уколов]

Слушать все клики по документу - не лучшая идея, во-первых, это ресурсозатратно, а во-вторых, где-то посреди события может затесаться event.stopPropogation() и все сломается.

Answer 2

[Алексей Уколов]

У вас по сути N систем из трех взаимосвязанных элементов.
Сами системы между собой никак не связаны, а вот элементы внутри системы между собой нужно как-то связать.
Самый простой вариант - использовать индексы (интерактивный пример):

var buttons = document.querySelectorAll('.js-btn-item'),
    popups = document.querySelectorAll('.js-products-popup'),
    closers = document.querySelectorAll('.js-btn-close'),
    index, button, popup, closer;

for (index = 0; index < buttons.length; index++) {
    button = buttons[index];
    popup = popups[index];
    closer = closers[index];

    button.addEventListener('click', buttonHandler(popup));
    closer.addEventListener('click', closerHandler(popup))
}

function buttonHandler(popup) {
    return function (event) {
        event.preventDefault();
        popup.classList.add('js-popup-show');
    }
}

function closerHandler(popup) {
    return function (event) {
        event.preventDefault();
        popup.classList.remove('js-popup-show');
    }
}

Объяснение, зачем мы делаем такие сложные обработчики событий из двух вложенных функций, находится здесь. В современных браузерах можно сделать проще, но при этом внутри обработчика события this будет ссылаться уже не на элемент, а на что-то другое (скорее всего, window), но в данном случае это не имеет значения.

Для того, чтобы этот вариант работал, естественно, элементы каждого типа должны идти в строго одинаковом порядке.
Чтобы этого избежать, можно использовать родителя-обертку, внутри которого содержатся все 3 элемента системы (пример). Такой подход, помимо всего прочего, за счет делегирования позволяет устанавливать N обработчиков, а не N*2, как в первом случае. Но код обработчика слегка усложняется - добавляется проверка целевого элемента.

Третий способ показал Алексей Зуев - он более декларативный, как видите, javascript-кода там совсем немного - одна универсальная функция. Но этот вариант требует использования id вместо классов, это не всегда возможно.