Optimize adding events to various Javascript elements

Asked

Viewed 92 times

1

I have the following question: How to optimize the script below? In total there are 14 clicks and each changes a certain ID. I don’t want to use jQuery.

Script that adds the listeners event:

document.getElementById("btn-1").addEventListener("click", function () {
  myFunction(1);
});
document.getElementById("btn-2").addEventListener("click", function () {
  myFunction(2);
});
document.getElementById("btn-3").addEventListener("click", function () {
  myFunction(3);
});
document.getElementById("btn-4").addEventListener("click", function () {
  myFunction(4);
});

Script that changes the styles according to the value passed:

function myFunction(a) {
  if (a === 1) {
    document.getElementById("cid-1").style.display = 'block';
    document.getElementById("area-1").style.fill = '#dfdca3';
  } else if (a === 2) {
    document.getElementById("cid-2").style.display = 'block';
    document.getElementById("area-2").style.fill = '#ed694b';
  } else if (a === 3) {
    document.getElementById("cid-3").style.display = 'block';
    document.getElementById("area-3").style.fill = '#5fa7c8';
  } else if (a === 4) {
    document.getElementById("cid-4").style.display = 'block';
    document.getElementById("area-4").style.fill = '#cce2e8';
  }
}

3 answers

2

Using "variables", example:

function myFunction(a) {
    var cid, color;

    if (a === 1) {
      cid = a;
      color = '#dfdca3';
    } else if (a === 2) {
      cid = a;
      color = '#ed694b';
    } else if (a === 3) {
      cid = a;
      color = '#5fa7c8';
    } else if (a === 4) {
      cid = a;
      color = '#cce2e8';
    }

    if (cid) {
       document.getElementById("cid-" + cid).style.display = 'block';
       document.getElementById("area-" + cid).style.fill = '#cce2e8';
    }
}

Not necessarily optimizes, but decreases the code.

Delegating events

However adding this to 14 buttons is what should be your real problem, so in this case you could opt for delegation of events, would look something like:

var ids = [1, 2, 3, 4]; //Botões por numeros apenas

//Unico evento necessário aplicado ao document
document.addEventListener('click', function(e) {

    //Checa se o click no document veio de um dos botões
    var correto = e.target.matches('#btn-' + ids.join(', #btn-'));

    //Se veio de um dos botões pega o ID remove a parte btn- e passa para a função o valor
    if (correto) {
        myFunction(parseInt(e.target.id.replace(/^btn-/, '')));
    }
});

The great advantage of this over using directly to the elements is that you don’t need to use window.onload or DOMContentLoaded and not even check if the element exists, because it already circumvents all this, including if removing the element does not need to use the removeEventListener.

  • Massa! already gives a "dry" in the codes. I will give a study in this stop delegating events. Valew msm!!

  • @Dmiaguy The great advantage of this over applying directly to the elements is that you do not need to use window.onload or Domcontentloaded and do not even check if the element exists, because it already circumvents all this, including whether to remove the element or need to use removeEventListener.

2


In relation to listeners event, you can create a array with numbers and get the Ids through a forEach. Something like that:

[1, 2, 3, 4].forEach((num) => {
  document.getElementById(`btn-${num}`).addEventListener('click', () => {
    myFunction(num);
  });
});

With this you reduce N explicit calls to N implicit calls, since you used a array with forEach to remove unnecessary code.


Now you can modify the function myFunction to avoid the extensive number of conditions. First, you can get the elements #cid-* and #area-* dynamically through the parameter num which is received. You can also use an object to create a map type by associating each number with a specific hexadecimal color. Something like this:

function myFunction(num) {
  const cidEl = document.getElementById(`cid-${num}`);
  const areaEl = document.getElementById(`area-${num}`);

  const map = {
    1: '#dfdca3',
    2: '#ed694b',
    3: '#5fa7c8',
    4: '#cce2e8'
  };

  // Obtemos o valor hexadecimal:
  const hexCode = map[num];

  // Verificação para saber se temos um item correspondente no mapa,
  // e os elementos foram encontrados. Caso não, simplesmente não fará nada.
  if (!cidEl || !areaEl || !hexCode) {
    return; // Você pode tratar de forma diferente, se desejar.
  }

  cidEl.style.display = 'block';
  areaEl.style.fill = hexCode;
}

For a deeper understanding

  • Got it!! I’ll give a study in your code to learn. But from what I saw he meets and very well. Thank you very much!

0

Instead of using id’s in sequence, I would suggest using class. So you’ll just need to put the same class on each button and another class in each element you want to show, and to search within that element the element wants to apply the fill. Just use the querySelectorAll().

Here the function of forEach returns two variables (el and a), where the first is the element itself and the second the index (the first element has index 0, the second 1 and so on), and you pass the index of the clicked button as parameter to the other function myFunction():

document.querySelectorAll(".btns").forEach(function(el,a){
   el.addEventListener("click", function () {
      myFunction(a);
   });
});

The buttons would have class .btns (no need for id’s):

<button class="btns">btn-1</button>
<button class="btns">btn-2</button>
<button class="btns">btn-3</button>
<button class="btns">btn-4</button>

How each index of a button will relate to each element to be shown (style.display = 'block'), just use the querySelectorAll(classe)[índice] to show the widget and find inside it the widget that will receive Fill.

In the example below, each svg has the same class .cid. The first has index 0, the second 1 and so on, just like the buttons. Like the function myFunction(a) will receive the index of the button clicked, just fetch the same svg which has the same index within the class .cid. In my view it is an advantage the use of class because it makes the code a little more flexible and lean:

document.querySelectorAll(".btns").forEach(function(el,a){
   el.addEventListener("click", function () {
      myFunction(a);
   });
});

function myFunction(a) {

   const cores = {
      0: 'dfdca3',
      1: 'ed694b',
      2: '5fa7c8',
      3: 'cce2e8'
   }

   let svg = document.querySelectorAll(".cid")[a];

   svg.style.display = 'block';
   svg.querySelector("circle").style.fill = '#'+cores[a];
}
svg{
   display: none;
}
<button class="btns">btn-1</button>
<button class="btns">btn-2</button>
<button class="btns">btn-3</button>
<button class="btns">btn-4</button>
<svg class="cid" width="100" height="100">
   <circle cx="50" cy="50" r="40" />
</svg>
<svg class="cid" width="100" height="100">
   <circle cx="50" cy="50" r="40" />
</svg>
<svg class="cid" width="100" height="100">
   <circle cx="50" cy="50" r="40" />
</svg>
<svg class="cid" width="100" height="100">
   <circle cx="50" cy="50" r="40" />
</svg>

  • opa! Interesting not to have to put id on everything and yes pull by the class, I will study here to see how it looks. Thank you very much!

Browser other questions tagged

You are not signed in. Login or sign up in order to post.