Why is the function in my Javascript code executing right away but not when I click the button?

Question

I am having trouble understanding why the text is not changing when I click the button. It is being executed right away when the page starts instead. I am not sure why this is happening because I told it to only execute when you click on the button.

<!DOCTYPE html>
<html id="all">
<head>
    <head>
    </head>
    <title>Lab8</title>

    <style></style>

    </head>

<body>
<script>
    var iDiv = document.createElement('div');
    iDiv.id = 'block';
    iDiv.className = 'block';
    document.getElementsByTagName('body')[0].appendChild(iDiv);
    iDiv.style.backgroundColor = "#d79365";
    iDiv.style.padding = "40px";

    var innerDiv2 = document.createElement('div');
    innerDiv2.className = 'block-3';

    iDiv.appendChild(innerDiv2);
    innerDiv2.style.padding = "40px";
    innerDiv2.style.textAlign = "center";
    innerDiv2.innerHTML = "Here is changing the text: ";
    //innerDiv2.innerHTML = "Text Change when button clicked";
    //innerDiv2.style.textAlign = "center";



    // Now create and append to iDiv
    var innerDiv = document.createElement('div');
    innerDiv.className = 'block-2';

    // The variable iDiv is still good... Just append to it.
    iDiv.appendChild(innerDiv);
    innerDiv.innerHTML = "I'm the inner div";
    innerDiv.style.padding = "40px";
    innerDiv.style.backgroundColor = "#eac67a";

    var ClickButton = document.createElement('button');
    ClickButton.className = 'block-4';
    iDiv.appendChild(ClickButton);
    ClickButton.innerHTML = "Style";
    ClickButton.style.margin = "auto";
    ClickButton.style.display = "block";
    ClickButton.style.width = "80px";
    ClickButton.style.height = "50px";
    ClickButton.style.top = "50px";
    ClickButton.style.backgroundColor = "#233237";
    ClickButton.style.color = "white";

    function js_style(){
        alert("hi");
        document.querySelector("innerDiv2");
        innerDiv2.style.fontSize = 'large';
        innerDiv2.style.font = 'italic bold 20px arial,serif';
        innerDiv2.style.color = "#eac67a";
    };

    document.getElementsByTagName('button').onclick = js_style();

    </script>
</body>

You have already correctly used `document.getElementsByTagName('body')[0]` (although `document.body` is shorter). Why do you still have a mistake at `document.getElementsByTagName('button').onclick`? `document.getElementsByTagName` doesn’t return a single element. — Sebastian Simon, Mar 04 '17 at 13:59
you create the button `ClickButton` ... use `ClickButton.addEventListener('click', js_style)` and forget the old school "onclick" rubbish — Jaromanda X, Mar 04 '17 at 14:00
Please remove the unnecessary bits, like all the style stuff-it's hard to see what's important. — Dave Newton, Mar 04 '17 at 14:02
Possible duplicate of [What do querySelectorAll, getElementsByClassName and other getElementsBy\* methods return?](http://stackoverflow.com/questions/10693845/what-do-queryselectorall-getelementsbyclassname-and-other-getelementsby-method) — Sebastian Simon, Mar 04 '17 at 14:19

GitProphet · Answer 1 · 2017-03-04T14:06:52.740

document.getElementsByTagName('button') returns a HTMLCollection. Setting the 'onclick' value of the HTMLCollection does not set the 'onclick' handle of the button.

i could get the example to work by giving the button an id and retrieving the button via that id (rather than it's tag name): https://jsfiddle.net/0L1kj3ja/

var iDiv = document.createElement('div');
    iDiv.id = 'block';
    iDiv.className = 'block';
    document.getElementsByTagName('body')[0].appendChild(iDiv);
    iDiv.style.backgroundColor = "#d79365";
    iDiv.style.padding = "40px";

    var innerDiv2 = document.createElement('div');
    innerDiv2.className = 'block-3';

    iDiv.appendChild(innerDiv2);
    innerDiv2.style.padding = "40px";
    innerDiv2.style.textAlign = "center";
    innerDiv2.innerHTML = "Here is changing the text: ";
    //innerDiv2.innerHTML = "Text Change when button clicked";
    //innerDiv2.style.textAlign = "center";



    // Now create and append to iDiv
    var innerDiv = document.createElement('div');
    innerDiv.className = 'block-2';

    // The variable iDiv is still good... Just append to it.
    iDiv.appendChild(innerDiv);
    innerDiv.innerHTML = "I'm the inner div";
    innerDiv.style.padding = "40px";
    innerDiv.style.backgroundColor = "#eac67a";

    var ClickButton = document.createElement('button');
    ClickButton.id = 'btn';
    ClickButton.className = 'block-4';
    iDiv.appendChild(ClickButton);
    ClickButton.innerHTML = "Style";
    ClickButton.style.margin = "auto";
    ClickButton.style.display = "block";
    ClickButton.style.width = "80px";
    ClickButton.style.height = "50px";
    ClickButton.style.top = "50px";
    ClickButton.style.backgroundColor = "#233237";
    ClickButton.style.color = "white";

    function js_style(){
        alert("hi");
        document.querySelector("innerDiv2");
        innerDiv2.style.fontSize = 'large';
        innerDiv2.style.font = 'italic bold 20px arial,serif';
        innerDiv2.style.color = "#eac67a";
    };

    document.getElementById('btn').onclick = js_style;

and, just for fun, and who knows why, but by adding an id, you can do `document.getElementsByTagName('button').btn.onclick = js_style;` !! — Jaromanda X, Mar 04 '17 at 14:11

score 1 · Answer 2 · answered Mar 04 '17 at 14:03

The problem with your code is that getElementsByTagName returns a HTMLCollection - which behaves a little like an array, in that you can access the individual elements using array syntax like x[0]

However, as you're creating the button dynamically, you can dispense with that, and, in the process, dispense with last millennium code element.onclick=rubbish

var ClickButton = document.createElement('button');
ClickButton.addEventListener('click', js_style);

done

pouyan · Answer 3 · 2017-03-04T14:15:15.063

0

instead of document.getElementsByTagName('button').onclick = js_style(); try this :

 var buttons = document.getElementsByTagName('button');
for (var i=0;i<buttons.length;i++)
{
buttons[i].onclick = js_style;
}

when you use js_style(); javascript will call that function so you should just introduce your function name to .onclick

edited Mar 04 '17 at 14:15

answered Mar 04 '17 at 13:54

pouyan

3,445
4
26
44

@user2891660 how about my new approach? – pouyan Mar 04 '17 at 14:00
2

you still don't get what `getElementsByTagName` returns ... hint: elementS – Jaromanda X Mar 04 '17 at 14:01
@JaromandaX i edited my answer. i think it would work now, am i right? – pouyan Mar 04 '17 at 14:09

Kalpesh Singh · Answer 4 · 2017-03-04T14:36:55.963

You were attaching onclick event to collection of JS nodes. I've refactored your code and added one more class to button element to attachclick event on that.

It is always advisable to add class with js prefix to attach event handler to DOM elements. In this way,no one will mess with js-* classes.

//create outer div and apply styles
var outerDiv = document.createElement('div');
var outerDivStyle = 'background-color:#d79365;padding: 40px;';
outerDiv.id = 'block';
outerDiv.className = 'block';
document.getElementsByTagName('body')[0].appendChild(outerDiv);
outerDiv.style.cssText = outerDivStyle;

//create inner div and apply styles
var innerDiv = document.createElement('div');
innerDiv.className = 'block-3';

outerDiv.appendChild(innerDiv);
var innerDivStyle = 'padding: 40px;text-align:center;';
innerDiv.innerHTML = "Here is changing the text: ";
innerDiv.style.cssText = innerDivStyle;



//create last div and apply styles
var lastDiv = document.createElement('div');
lastDiv.className = 'block-2';

// The variable iDiv is still good... Just append to it.
outerDiv.appendChild(lastDiv);
lastDiv.innerHTML = "I'm the inner div";
var lastDivStyle = 'background-color:#eac67a;padding: 40px;';
lastDiv.style.cssText = lastDivStyle;


//create button
var clickButton = document.createElement('button');
clickButton.id = 'js-btn';
clickButton.className = 'block-4';
outerDiv.appendChild(clickButton);
var btnStyles = 'background-color:#233237;color: white;margin:auto;display:block;width: 80px; height:50px;';
lastDiv.style.cssText = lastDivStyle;
clickButton.innerHTML = "Style";
clickButton.style.cssText = btnStyles;

function jsStyle() {
  alert("hi");
  document.querySelector("innerDiv");
  innerDiv.style.fontSize = 'large';
  innerDiv.style.font = 'italic bold 20px arial,serif';
  innerDiv.style.color = "#eac67a";
};

document.querySelector('#js-btn').onclick = jsStyle;

_“It is always advisable to add class with `js` prefix to attach event handler to DOM elements. In this way, you will increase performance a bit and no one will mess with `js-*` classes.”_ — Citation needed. — Sebastian Simon, Mar 04 '17 at 14:23
This is from my personal knowledge.Though, you might find these articles useful. More about prefixes is here - https://www.smashingmagazine.com/2016/06/battling-bem-extended-edition-common-problems-and-how-to-avoid-them/ I couldn't find any citation for performance of selector so editing it. — Kalpesh Singh, Mar 04 '17 at 14:36

Why is the function in my Javascript code executing right away but not when I click the button?

4 Answers4