Tomas Sereičikas
Tomas Sereičikas

Reputation: 203

How to make JavaScript Event Listeners less repetitive?

I am having some trouble thinking new ways to optimize this block of code. Now it looks too repetitive and long. I can write functions separately from EventListener but it would only make more lines of code and make it even longer.

let modalIntro = document.getElementById('modal-intro');
let buttonIntro = document.getElementById('button-intro');
let close = document.getElementsByClassName('close')[0];

buttonIntro.addEventListener ('click', function(){
    modalIntro.classList.remove("out");
    modalIntro.classList.add("in");
});

close.addEventListener('click', function (){
    modalIntro.classList.add("out");
});

let modalWork = document.getElementById('modal-work');
let buttonWork = document.getElementById('button-work');
let close1 = document.getElementsByClassName('close')[1];

buttonWork.addEventListener ('click', function(){
    modalWork.classList.remove("out");
    modalWork.classList.add("in");
});

close1.addEventListener('click', function (){
    modalWork.classList.add("out");
});

let modalAbout = document.getElementById('modal-about');
let buttonAbout = document.getElementById('button-about');
let close2 = document.getElementsByClassName('close')[2];

buttonAbout.addEventListener ('click', function(){
    modalAbout.classList.remove("out");
    modalAbout.classList.add("in");
});

close2.addEventListener('click', function (){
    modalAbout.classList.add("out");
});

let modalContact = document.getElementById('modal-contact');
let buttonContact = document.getElementById('button-contact');
let close3 = document.getElementsByClassName('close')[3];

buttonContact.addEventListener ('click', function(){
    modalContact.classList.remove("out");
    modalContact.classList.add("in");
});

close3.addEventListener('click', function (){
    modalContact.classList.add("out");
});

Any help would be much appreciated.

Upvotes: 0

Views: 263

Answers (5)

RobIII
RobIII

Reputation: 8831

Easiest would be to simply create a function:

function MakeModal(modalId, buttonId, closeId)
    let modal = document.getElementById(modalId);
    let button = document.getElementById(buttonId);
    let close = document.getElementsById(closeId);

    button.addEventListener ('click', function(){
        modal.classList.remove("out");
        modal.classList.add("in");
    });

    close.addEventListener('click', function (){
        modal.classList.add("out");
    });
}

Which you can then invoke:

MakeModal('modal-intro', 'button-into', 'close-intro');
MakeModal('modal-about', 'button-about', 'close-about');
MakeModal('modal-contact', 'button-contact', 'close-contact');

That's just programming basics: DRY.

Do note that you need to add ID's to your close-buttons instead of classes (or you can rewrite it to let close = modal.find('.close'); or something and in which case you can get rid of the third argument closeId). And if you adhere strictly to the naming convention used here you could even simplify the multiple MakeModal(...) calls:

['intro', 'about', 'contact'].forEach(function(e) { 
    MakeModal('modal-' + e, 'button-' + e, 'close-' + e); 
});

Or, if you got rid of the third argument and used the .find(...) suggestion:

['intro', 'about', 'contact'].forEach(function(e) { 
    MakeModal('modal-' + e, 'button-' + e); 
});

I'm no jQuery expert by any means but if I'm not mistaken

modal.classList.remove("out");
modal.classList.add("in");

can be written as:

modal.classList.switchClass("out", "in");

Upvotes: 0

user5734311
user5734311

Reputation:

Here's another approach that uses the fact that you have the same structure times three. This would be much shorter if I had used jQuery, but I kept it vanilla since you don't seem to be using it:

document.querySelectorAll("nav a").forEach(a => a.addEventListener("click", function(e) {
  e.preventDefault(); // don't navigate to href
  // hide all modals, i.e. remove the one potentially still open
  document.querySelectorAll("#modals > div").forEach(modal => modal.classList.remove("in"));
  // finally, show modal
  document.getElementById(this.dataset.modal).classList.add("in");
}));

document.querySelectorAll("#modals > div").forEach(modal =>
  // for each modal, grab its close button
  modal.querySelector(".close").addEventListener("click", function(e) {
    e.preventDefault(); // don't navigate to href
    modal.classList.remove("in"); // and hide the modal
  })
);
#modals>div {
  position: fixed;
  width: 50%;
  left: 25%;
  box-sizing: border-box;
  border: 1px solid black;
  padding: 0.5em;
  top: -300px;
  transition: .5s
}

#modals .close {
  float: right
}

.in {
  top: 50px !important;
}
<nav>
  <a data-modal="intro" href="">Intro</a>
  <a data-modal="work" href="">Work</a>
  <a data-modal="about" href="">About</a>
</nav>
<div id="modals">
  <div id="intro"><a href="" class="close">X</a>
    <p>Intro modal</p>
  </div>
  <div id="work"><a href="" class="close">X</a>
    <p>Work modal</p>
  </div>
  <div id="about"><a href="" class="close">X</a>
    <p>About modal</p>
  </div>
</div>

By pointing the code to the modal via the menu link's data-modal, I no longer have to use any element-specific code at all, resulting in ideally reusable code.

Upvotes: 0

AnonymousSB
AnonymousSB

Reputation: 3594

Seeing as you have a common naming pattern, and assuming the close button is child of each modal, you could do something like this:

Note I added modal.classList.remove('in'); to the close button

Solution

function bindModals(modals) {
  modals.forEach(name => {
    let modal = document.getElementById(`modal-${name}`)
    let button = document.getElementById(`button-${name}`)
    let close = modal.querySelector('.close');

    button.addEventListener ('click', function(){
      modal.classList.remove('out');
      modal.classList.add('in');
    });
    close.addEventListener('click', function (){
      modal.classList.remove('in');
      modal.classList.add('out');
    });
  });
}
bindModals(['intro', 'work', 'about', 'contact'])
.out {
  display: none;
}
.in {
  display: block;
  border: 1px solid red;
  height: 200px;
  width: 400px;
}
<section>
  <button id="button-intro">Intro</button>
  <button id="button-work">Work</button>
  <button id="button-about">About</button>
  <button id="button-contact">Contact</button>
</section>
<section id="modal-intro" class="out">
  <button class="close">Close</button>
  <p>Intro Modal</p>
</section>
<section id="modal-work" class="out">
  <button class="close">Close</button>
  <p>Work Modal</p>
</section>
<section id="modal-about" class="out">
  <button class="close">Close</button>
  <p>About Modal</p>
</section>
<section id="modal-contact" class="out">
  <button class="close">Close</button>
  <p>Contact Modal</p>
</section>

Upvotes: 1

user6420561
user6420561

Reputation:

Without having any HTML I am not sure if this is going to be perfectly accurate but I can try something.

I see each modal behaves in the same way... I would add an attribute to each button that indentifies which modal it activates, and a class so we can refer to all of them:

<input type="button" value="intro" class="modalButton" id="button-intro" data-activates="modal-intro">
<input type="button" value="work" class="modalButton" id="button-work" data-activates="modal-work">
<input type="button" value="about" class="modalButton" id="button-about" data-activates="modal-about">
<input type="button" value="contact" class="modalButton" id="button-contact" data-activates="modal-contact">

Now we can refer to each and action with this line of code:

document.getElementsByClassName("modalButton").forEach(function(btn) {
    btn.addEventListener ('click', function() {
        var modal = document.getElementById(btn.getAttribute("data-activates"));
        modal.classList.remove("out");
        modal.classList.add("in");
    });
});

Assuming the buttons are direclty inside the modal like this:

<!-- Example of a modal with a close button -->
<div id="modal-intro" class="modal out">
    <input type="button" class="close" value="close">
    <!-- content -->
</div>

We can get to the modal by moving up in the DOM tree:

document.getElementsByClassName("close").forEach(function(closeBtn) {
    closeBtn.addEventListener ('click', function(){
        // In this case the modal is the button's parent
        closeBtn.parentElement.classList().remove("in");
        closeBtn.parentElement.classList().add("out");
        // P.S I also suppose you want to remove the "in" class
    });
});

Obiouvsly, if the button is deeper inside the modal, you just need to call the 'parentElement' property till you get to it.

Upvotes: 0

arun
arun

Reputation: 55

You can use a function to get element Id to make it more shorter:

function getElem(el) {
  return document.getElementById(el);
}

Now remove all below code:

let modalIntro = document.getElementById('modal-intro');
let buttonIntro = document.getElementById('button-intro');

let modalWork = document.getElementById('modal-work');
let buttonWork = document.getElementById('button-work');


let modalContact = document.getElementById('modal-contact');
let buttonContact = document.getElementById('button-contact');

– and replace variable with getElem('elem-id'), e.g given below

    getElem('modal-intro').classList.remove("out");
    getElem('modal-intro').classList.add("in");

Similarly you can make common function to get Element by class and add event listener.

Upvotes: 0

Related Questions