Your problem was that you forgot to add ()
after your function name.
Beyond that, there are a few other things to correct:
Don't use inline HTML event attributes (onclick
, onmouseover
, etc.) as they:
- Create "spaghetti code" that is difficult to read and debug.
- Lead to duplication of code.
- Don't scale well
- Don't follow the separation of concerns development methodology.
- Create anonymous global wrapper functions around your attribute values that alter the
this
binding in your callback functions.
- Don't follow the W3C Event Standard.
- Don't cause a reference to the DOM event to be passed to the handler.
Even MDN agrees
Inline event handlers — don't use these
You might also see a pattern like this in your code:
<button onclick="bgChange()">Press me</button>
function bgChange() {
const rndCol = `rgb(${random(255)}, ${random(255)}, ${random(255)})`;
document.body.style.backgroundColor = rndCol;
}
The earliest method of
registering event handlers found on the Web involved event handler
HTML attributes (or inline event handlers) like the one shown above —
the attribute value is literally the JavaScript code you want to run
when the event occurs. The above example invokes a function defined
inside a element on the same page, but you could also insert
JavaScript directly inside the attribute, for example:
<button onclick="alert('This is my old-fashioned event handler!');">Press me</button>
You can find HTML attribute
equivalents for many of the event handler properties; however, you
shouldn't use these — they are considered bad practice. It might seem
easy to use an event handler attribute if you are doing something
really quick, but they quickly become unmanageable and inefficient.
For a start, it is not a good idea to mix up your HTML and your
JavaScript, as it becomes hard to read. Keeping your JavaScript
separate is a good practice, and if it is in a separate file you can
apply it to multiple HTML documents.
Even in a single file, inline event handlers are not a good idea. One
button is OK, but what if you had 100 buttons? You'd have to add 100
attributes to the file; it would quickly turn into a maintenance
nightmare. With JavaScript, you could easily add an event handler
function to all the buttons on the page no matter how many there were,
using something like this:
const buttons = document.querySelectorAll("button");
for (const button of buttons) {
button.addEventListener("click", bgChange);
}
Finally, many common server configurations will disallow
inline JavaScript, as a security measure.
You should never use the HTML event handler attributes — those are
outdated, and using them is bad practice.
Instead, do all your work in JavaScript and use .addEventListener()
to set up event handlers.
Don't use a hyperlink when a button (or some other element) will do. If you do use a hyperlink, you need to disable its native desire to navigate, which is done by setting the href
attribute to #
, not ""
.
// Place this code into a <script> element that goes just before the closing body tag (</body>).
// Get a reference to the button and the iframe
var btn = document.getElementById("btnChangeSrc");
var iFrame = document.getElementById("contentFrame");
// Set up a click event handler for the button
btn.addEventListener("click", getContent);
function getContent() {
console.log("Old source was: " + iFrame.src);
iFrame.src="LoremIpsum.html";
console.log("New source is: " + iFrame.src);
}
<div class="sidebar"><h1>Technical Documentation</h1>
<ul>
<li>Configuration Guides</li>
<li>API Guides</li>
</ul>
<button id="btnChangeSrc">Change Source</button>
</div>
<iframe class="content" id="contentFrame" src="dummy.html"></iframe>