Modal box isn't working properly

Question

I am writing a small webapp and I decided I will not use any framework. I will write only vanilla javascript.

The problem I have is that I want to be able to open three different types of modal windows with different keyboard keys. For example, g will open one type, d will open another, and m yet another.

I tried to implement the g one, which will contain a title and an input form, and the d one, with a title and two buttons. Here is my code so far:

var width = 720;
var height = 350;
var modal = document.createElement('div');
modal.className  = "modal";
modal.style.width = width;
modal.style.height = height;
modal.style.position = 'absolute';
modal.style.top = (window.innerHeight - parseInt(modal.style.height)) / 2 +'px';
modal.style.left = (window.innerWidth - parseInt(modal.style.width)) / 2 +'px';

var ngroup = modal;
var title = document.createElement('span');
title.innerHTML = "TYPE GROUP NAME"
var input = document.createElement('input');
input.type = "text";
ngroup.appendChild(title);
ngroup.appendChild(input);

var del = modal;
var dtitle = document.createElement('span');
dtitle.innerHTML = "DO YO WANT TO DELETE SELECTED COLOR(S)"
var dinput = document.createElement('input');
dinput.type = "button";
dinput.value = "yes";
var dinput1 = document.createElement('input');
dinput1.type = "button";
dinput1.value = "no";
del.appendChild(dtitle);
del.appendChild(dinput);
del.appendChild(dinput1);   
function openModal(type) {
  var body = document.getElementsByTagName('body')[0];
  body.appendChild(type);
}
function closeModal() {
  document.body.removeChild(modal);
}

and this is modal.js, which is included in the main page and called by this:

document.addEventListener('keydown', function(event) {
    if(event.keyCode == 71) {
        openModal(del);
    }
    else if(event.keyCode == 39) {
        closeModal();
    }
});

When I hit g, it opens a modal with every element: two titles, one text field and two buttons. Why? What is the problem with my Javascript? I know it’s still not organized, but I’d like to make things work before getting into that.

Answer 1

Even if you aren't using a library, I would recommend using the same tactic as jQuery. Specifically, I would specify all three dialogs in the static HTML in a hidden div. For example, the following would be the 'd' dialog:

<div id="d_dialog" style="display:none" class="dialog">
    <span>DO YO WANT TO DELETE SELECTED COLOR(S)</span>
    <input type="button" value="yes"/>
    <input type="button" value="no"/>
</div>

Then, from JavaScript when you want to open the dialog, just show the hidden div.

document.getElementById('d_dialog').style.display = '';

This will make your work much more maintainable than creating all the HTML elements from JavaScript. You can specify the style of the dialog by adding a class called dialog to your CSS.

The specific problem with your code

The specific problem with your code is you are reassigning the same variable modal to multiple other variables.

var del = modal;

This doesn't make a copy of the modal element. It just makes the new variable del reference the same object as the modal variable. Therefore when you call:

del.appendChild(dtitle);

It behaves the same as if you had called

modal.appendChild(dtitle);

So all the controls are added to the modal element. And when you show the dialog with

opendDialog(del);

It behaves as if you had called

openDialog(modal);

And shows the element with all the controls on it. To make your code work, you should create a function createModal that just does the following:

function createModal()
{
    var width = 720;
    var height = 350;
    var modal = document.createElement('div');
    modal.className  = "modal";
    modal.style.width = width;
    modal.style.height = height;
    modal.style.position = 'absolute';
    modal.style.top = (window.innerHeight - parseInt(modal.style.height)) / 2 +'px';
    modal.style.left = (window.innerWidth - parseInt(modal.style.width)) / 2 +'px';
    return modal;
}

Then, for each of the dialogs, instead of

var del = modal;

Do

var del = createModal();

This will create a new element instead of creating a new variable that refers to the same element.