Как правильно избавиться от дублирования js кода?

Question

[testkeyc]

Пишу всплывающее окно на сайте. Есть две переменные, которые подключаются по клику к id. По клику добавляется display: block для элемента. Подскажите, как правильно переписать функцию, чтобы избежать дублирования кода для одного действия?

let modal = document.getElementById('modal');
let modal2 = document.getElementById('modal');

link.onclick = function() {
    modal.style.display = "block";
}

link2.onclick = function() {
    modal2.style.display = "block";
}

Answer 1

[Andrey Suha]

function show(modal) {
    modal.style.display = "block";
}

link.onclick = show(modal);
link2.onclick = show(modal2);

upd

function show(modalId) {
    let modal = document.getElementById(modalId);
    modal.style.display = "block";
}

link.onclick = show('modal');
link2.onclick = show('modal2');

Comments

[Антон Швец]

вы в onclick передаете undefined

[Andrey Suha]

Антон Швец, можно подробнее?

[Andrey Suha]

Антон Швец, все понял, действительно

[Антон Швец]

Andrey Suha, link.onclick = show('modal');
тут в переменную link.onclick передаешь результат выполнения show('modal')
А что возвращает эта фукнция? Ничего.

Надо
link.onclick = () => show('modal');
Учите js перед тем, как отвечать.

Answer 2

[Антон Швец]

function getClickFunction(elementId) {
  return () => document.getElementById(elementId).style.display = "block";
}
link.onclick = getClickFunction('modal');
link2.onclick = getClickFunction('modal2');

Answer 3

[dollar]

Универсальный способ в лоб - взять и создать отдельную функцию.

Код

let modal = document.getElementById('modal');
let modal2 = document.getElementById('modal2');

function commonFn(e) {
    e.style.display = "block";
}

link.onclick = function() {
    commonFn(modal);
}

link2.onclick = function() {
    commonFn(modal2);
}

Более хитрый, но странный способ - каким-то образом получить массив элементов. Способы есть разные, не о них речь.

Код

let modal = document.getElementById('modal');
let modal2 = document.getElementById('modal2');

let links = [link, link2];
let modals = [modal, modal2];

links.forEach((L,i)=>L.onclick = function() {
    modals[i].style.display = "block";
});

К слову, вместо .onclick= лучше использовать .addEventListener('click',

Ну и наиболее правильный, как мне кажется, способ - это каким-то образом связать каждый link с соответствующим modal. Выше эта связь происходит через индекс в массиве. Но может быть, что можно как-то по-другому, через цепочку соседних или родительски-дочерних элементов вычислить. В идеале у link и modal должны быть похожие атрибуты, с помощью которых функция в onclick, зная атрибут у link, сможет вычислить атрибут у modal, а затем найти его по этому атрибуту на странице. Например, одинаковый класс (один из).

Answer 4

[Eugene Chefranov]

Можно так
<a href="#my-modal" class="modal">Открыть окно</a>

$('.modal').on('click', function() {
var modal = $(this).attr(href);
$(modal).addClass('active');
})

.modal-window.active {display:block}

<div id="my-modal" class="modal-window">модальное окно</div>

Получается есть ссылка с классом modal и атрибутом href содержащий адрес модальной окна (id). Далее с помощью JS обрабатываем данную ссылку и на модальное окно вешаем класс active с стилем display:block. Получается минимум кода и управление переходит на HTML