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
– rootExplorr, Commented Jun 12, 2018 at 15:33

amrender singh · Accepted Answer · 2018-06-12 15:46:07Z

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, 2018 at 15:46

answered Jun 12, 2018 at 15:26

amrender singh

8,2594 gold badges27 silver badges30 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

dgeare Over a year ago

so, to be clear @RobertaGimenez in the console, you do NOT see and error? document.getElementByID is not a function?

Roberta Gimenez Over a year ago

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

kit Over a year ago

What kind of monsters will debug without a console?

Roberta Gimenez Over a year ago

HELP ME PLEASE.

Scott Marcus · Accepted Answer · 2018-06-12 15:50:15Z

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>

kit · Accepted Answer · 2018-06-12 15:37:49Z

-1

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);

answered Jun 12, 2018 at 15:37

kit

5655 silver badges11 bronze badges

1 Comment

Scott Marcus Over a year ago

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.

Collectives™ on Stack Overflow

onclick() with multiple function only execute the first

3 Answers 3

4 Comments

2 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related