onclick() with multiple function only execute the first

Question

I'm trying to execute two functions but it only execute the first one. I think that when I change a HTML variable value, the javascript stops its execution.

This is the code that I'm executing: HTML:

<div class="funcion" onclick="country_function();setGeo(1)">

JavaScript:

function country_function(){        
        region_hide();
        class_hide();
        country_show();
    };

function country_show(){
        var popup = document.getElementById("countrypopmenu");
        popup.classList.toggle("show");
    };
    function country_hide(){
        var popup = document.getElementById("countrypopmenu");
        popup.classList.toggle("hidden");
    };
    function region_show(){
        var popup = document.getElementById("regionpopmenu");
        popup.classList.toggle("show");
    };
    function region_hide(){
        var popup = document.getElementById("regionpopmenu");
        popup.classList.toggle("hidden");
    };
    function class_show(){
        var popup = document.getElementById("classpopmenu");
        popup.classList.toggle("show");
    };
    function class_hide(){
        var popup = document.getElementById("classpopmenu");
        popup.classList.toggle("hidden");
    };

    function setGeo(geoVal) {
      document.getElementByID('geodata').value= geoVal;
      window.alert(geoVal);
    };

Does anyone know why it doesn't execute all the functions?

Can you elaborate which functions are executing and which not? — rootExplorr, Jun 12 '18 at 15:33

amrender singh · Answer 1 · 2018-06-12T15:46:07.560

2

There is a typo in function satGeo :

 document.getElementById('geodata').value= geoVal;

function country_function(){        
        console.log("Inside function 1");
    };

  function setGeo(geoVal) {
    console.log("inside function 2");
  };

<div class="funcionff" onclick="country_function();setGeo(1);">click</div>

edited Jun 12 '18 at 15:46

answered Jun 12 '18 at 15:26

amrender singh

7,949
3
22
28

so, to be clear @RobertaGimenez in the console, you do NOT see and error? `document.getElementByID is not a function`? – dgeare Jun 12 '18 at 15:33
I'm not using a console so I only know that the value is not changing. I'm working in an app in my localhost – Roberta Gimenez Jun 12 '18 at 15:45
What kind of monsters will debug without a console? – kit Jun 12 '18 at 15:49
HELP ME PLEASE. – Roberta Gimenez Jun 12 '18 at 15:51

score 1 · Accepted Answer · answered Jun 12 '18 at 15:50

In setGeo, you've mis-capitalized .getElementById().

But, beyond that, you have a ton of code that is not-needed and a solution that is brittle and won't scale as your needs change.

You really just need one function that takes two arguments - what element to show/hide/toggle and what operation to perform on it (show/hide/toggle).

Additionally, the practice of setting up event handlers via HTML event attributes is a 25+ year old technique that dates back to before there were web standards and there are many reasons not to use this ancient technique today. Separate your JavaScript from your HTML completely and use modern standards for event configurations.

// Get your DOM references 
var f = document.querySelector(".function");
var c = document.querySelector(".country");
var r = document.querySelector(".region");
var l = document.querySelector(".locale");

// Set up your event handlers in JavaScript, not HTML
f.addEventListener("click", function(){
  // Just call the single function as many times as needed
  // with the appropriate arguments each time:
  showHideToggle(c,"show");
  showHideToggle(r,"hide");
  showHideToggle(l,"toggle");
});

// One function to do all operations
function showHideToggle(element, operation){
 // For show/hide/toggle operations just do the right
 // thing with the "hidden" class. 
 switch (operation.toLowerCase()) {
   case "show":
     element.classList.remove("hidden");
     break;
   case "hide":
     element.classList.add("hidden");
     break;
   default:
     element.classList.toggle("hidden");
     break;      
 }
}

div { opacity:1; transition:all 1s;  }
.function { cursor:pointer; user-select:none; margin:1em; }
.hidden { opacity:0; }

<div class="function">Click Me</div>
<div class="country hidden">Country</div>
<div class="region">Region</div>
<div class="locale hidden">Locale</div>

Ay muchisimas gracias – Roberta Gimenez Jun 12 '18 at 15:55 — Roberta Gimenez, Jun 12 '18 at 15:55
@RobertaGimenez De nada, – Scott Marcus Jun 12 '18 at 15:56 — Scott Marcus, Jun 12 '18 at 15:56

score -1 · Answer 3 · answered Jun 12 '18 at 15:37

I suggest to only attach one function to onclick attribute, then execute as many functions as needed in that event handler.

For example:

<div class="function" onclick="onFunctionButtonClick()"></div>

function onFunctionButtonClick () {
  region_hide();
  class_hide();
  country_show();
  setGeo(1);
}

Or even put the listener logic in JavaScript, so you can keep your HTML away from main application logic and have cleaner HTML.

<div class="function"></div>

var functionButton = document.getElementsByClassName('function')[0];
functionButton.addEventListener('click', onFunctionButtonClick);

`document.getElementsByClassName()` scans the entire document for all matching nodes, so it doesn't make sense to immediately throw out all but the first one after doing that. Additionally, it returns a "live" node list, which impedes performance. `.querySelector()` is what you want. — Scott Marcus, Jun 12 '18 at 15:54

onclick() with multiple function only execute the first

3 Answers3