Magne Raknaren Magne Raknaren - 3 months ago 6
HTML Question

JavaScript doesn't work after I remove one <div>

I am really new to JS and I'm having some issues.

So I have this JS file: that is basically the same function repeating with different

<div id="">
.

var button = document.getElementById("obj-trigger");

button.onclick = function () {
var div = document.getElementById("obj-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("lineas-trigger");

button.onclick = function() {
var div = document.getElementById("lineas-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("cultura-trigger");

button.onclick = function() {
var div = document.getElementById("cultura-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("igualdad-trigger");

button.onclick = function() {
var div = document.getElementById("igualdad-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("proyectos-trigger");

button.onclick = function() {
var div = document.getElementById("proyectos-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("estigmas-trigger");

button.onclick = function() {
var div = document.getElementById("estigmas-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("soy-trigger");

button.onclick = function() {
var div = document.getElementById("soy-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};

var button = document.getElementById("tudef-trigger");

button.onclick = function() {
var div = document.getElementById("tudef-cont");
if (div.style.display !== "none") {
div.style.display = "none";
}
else {
div.style.display = "block";
}
};


And it works perfectly when I am using ALL the functions, however if I remove a
<div>
from my HTML, let's say:
<div id="estigmas-trigger">
, my JS will work until it reaches:

var button = document.getElementById("estigmas-trigger");

button.onclick = function() {
var div = document.getElementById("estigmas-cont");
if (div.style.display !== "none") {
div.style.display = "none"
}
else {
div.style.display = "block";
}
};


All code below that will stop working, so no more collapsing. :(

Why is that? And... how can I fix it?

Answer

Your error is caused because when you remove an element from the HTML and then your Javascript tries to refer to that element without proper protections, it causes a script error and the script aborts execution because of the error.

The second thing you need to do when learning Javascript (after learning how to write your first script) is to learn how to check for errors in the debug console in the browser. That will show you when you have execution errors that are aborting your script and they will usually show you what line the error occurs on.

In this case, you would attempt to get a DOM element with a line such as:

var button = document.getElementById("estigmas-trigger");

And, then you would attempt to use the button variable. But, if the estigmas-trigger element was not in the page, then button would be null and it would be an error to reference a property of null such as .onclick.

In addition, your code is horribly repetitive. You really should never copy nearly identical code multiple times into your code. Instead, create a reusable function and use that function multiple places or if your code is almost entirely identical except for one input parameter (which is the case for you), then you can just put the one input parameter into an array and loop through the array.

Here's a much more DRY implementation (this replaces all of your code):

var buttons = ["obj-trigger", "lineas-trigger", "cultura-trigger", 
    "igualdad-trigger", "proyectos-trigger", "estigmas-trigger", 
    "soy-trigger", "tudef-trigger"];

buttons.forEach(function(id) {
    var button = document.getElementById(id);
    if (button) {
        button.addEventListener("click", function(e) {
            var cont_id = this.id.replace("trigger", "cont");
            var elem = document.getElementById(cont_id);
            if (elem) {
                var style = elem.style;
                if (style.display !== "none") {
                    style.display = "none";
                } else {
                    style.display = "block";
                }
            }
        });
    }
});

Summary of changes:

  1. Put all the trigger ID values into an array of strings so you can just loop through each one that you want to apply identical code to.

  2. Use .forEach() to loop through the array of strings.

  3. Get the DOM element for each id and check to see if it is present before trying to use it (this will solve your original problem).

  4. Use .addEventListener() to add the click event handler as this is much more scalable than .onclick because you can have multiple click handlers for the same element this way. It is a generally good habit to switch to use .addEventListener().

  5. Rather than refer to the xxx-cont ids by name, just derive them from the xxx-trigger ids using a simple text replacement. This saves more duplication and typing in your code.

  6. Get the xxx-cont object in the DOM and also check to see if it exists before attempting to use it (safe coding).