Plain vanilla JavaScript modal not working. I need another pair of eyes on this

Question

I am creating this in plain vanilla JavaScript.

I am trying to load modals by looping over a collection and I am only getting the last modal.

When I do it this way everything is fine.

buttons[0].onclick = function() {modals[0].style.display = "block";return false;}
buttons[1].onclick = function() {modals[1].style.display = "block";return false;}
buttons[2].onclick = function() {modals[2].style.display = "block";return false;}

When I do it this way I only get the last modal no matter which button I click

var i = 0;
while (i < buttons.length) {
  buttons[i].onclick = function() {modals[i].style.display = "block";return false;} 
  i++;
}

Any suggestions?

[Javascript infamous Loop issue?](https://stackoverflow.com/q/1451009) — VLAZ, Jul 20 '22 at 17:01
infamous loop issue. `buttons[i].onclick = function() { console.log(i); }` — epascarello, Jul 20 '22 at 17:01
These answers help but I have already created the buttons. I need to link existing buttons to existing modals. — DominicCapuano, Jul 20 '22 at 17:36

score 1 · Answer 1 · answered Jul 20 '22 at 19:15

1

It does seem to be the infamous look issue. This is what I came up with.

for (var i = 0, link; i < buttons.length; i++) {
    buttons[i].onclick = function(num) {
        return function() {
            modals[num].style.display = "block";
            return false;
        };
    }(i);
}

answered Jul 20 '22 at 19:15

DominicCapuano

25
2

This is a valid way to approach the problem. You may want to define a named function to generate the event listener rather than assigning it in each iteration of your loop. In addition, get in the habit of using `let` or `const` instead of `var`. `var` is a relic of early JS and really shouldn't be used anymore. – General Grievance Jul 21 '22 at 15:34

score 0 · Answer 2 · answered Jul 20 '22 at 18:18

Event delegation may be a solution for this kind of problems:

document.addEventListener(`click`, handle);

function handle(evt) {
  if (evt.target.dataset.modal) {
    document.querySelectorAll(`[data-modaldiv]`)
      .forEach(m => m.classList.remove(`active`));
    const divId = evt.target.dataset.modal  
    document.querySelector(`[data-modaldiv='${divId}']`)
      .classList.add(`active`);
  }
}

[data-modaldiv] {
  display: none;
}
[data-modaldiv].active {
  display: block;
}

<button data-modal="1">button 1</button>
<button data-modal="2">button 2</button>
<button data-modal="3">button 3</button>

<p data-modaldiv="1">Modal 1</p>
<p data-modaldiv="2">Modal 2</p>
<p data-modaldiv="3">Modal 3</p>

Plain vanilla JavaScript modal not working. I need another pair of eyes on this

2 Answers2