HTML code needs to be contained within HTML tags. For example:
<html>
<!-- html code inside here -->
</html>
The typical document structure that you’ll see is:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>The page title</title>
<link rel="stylesheet" href="path/to/css/file.css">
</head>
<body>
<h1>Heading of the document</h1>
<p>Content of the document</p>
<script src="path/to/script.js"></script>
</body>
</html>
The w3schools JavaScript section is still 15 years out of date. When they finally get time to adjust the pages to remove all of the code intended for Netscape 4 there will not be any inline scripts any more on their site as inline JavaScript is as dead as the Netscape 4 browser.
That w3schools site is the creation of some brothers, and has no official backing.
The advice given by that site was so bad that websites sprouted up to publicise those problems, to try and help educate people about the correct way of doing things.
Imagine a teacher that has students coming in saying that they don’t need much learning cause their daddy already taught them what they need to know. We here sometimes feel like that teacher.
Getting back to the topic at hand, the more correct way of achieving the above is to keep your JavaScript separate from your HTML, which can be easily done by placing an identifier on those buttons. You might also want some text on those buttons too to help people figure out what they are going to do too.
<button id="play">Play</button>
<button id="pause">Pause</button>
The HTML is now easy to manage and maintain, and the scripting can be easily attached to the buttons, regardless of whether it’s from the same file or from an external scripting file. Normally an external file is used to allow caching and performance benefits.
document.getElementById("play").onclick = function play() {
...
};
document.getElementById("pause").onclick = function pause() {
...
};
Your code uses a global variable called player, which hasn’t been defined. The browser will attempt to connect this with identifiers that have been defined in the HTML, but it’s better to avoid possible confusion and declare those variables properly.
It’s also preferred (for keeping code clear and easy to understand), to use ternary statements only for assigning values and not for calling functions. As such, an if statement is the better fit for this code.
document.getElementById("play").onclick = function play() {
var player = document.getElementById("player");
player.volume = 1.0;
player.play();
};
document.getElementById("pause").onclick = function pause() {
var player = document.getElementById("player");
player.volume = 1.0;
if (player.paused) {
player.play();
} else {
player.pause();
}
};
Things can be more correct than that. The onclick event is at risk of being replaced by other code, so to help protect yourself from that you can use addEventListener which allows multiple events (if need be) to be assigned to the one element.
We can also group the functions together, and the event assignments together, which makes it easier to manage future changes to the code.
function play() {
var player = document.getElementById("player");
player.volume = 1.0;
player.play();
}
function pause() {
var player = document.getElementById("player");
player.volume = 1.0;
if (player.paused) {
player.play();
} else {
player.pause();
}
}
document.getElementById("play").addEventListener("click", play, false);
document.getElementById("pause").addEventListener("click", pause, false);
If you want the code to be even more correct, so that a linter such as jslint.com will also like the code, we just need to tell jslint that the code is running in a browser, add the “use strict” directive which helps to ensure that certain bad habits aren’t allowed, and wrap the code in an IIFE (immediately invoked function expression) which helps to protect the browser from undesired side-effects in the code.
/*jslint browser */
(function iife() {
"use strict"
// rest of code here
}());
resulting in the more correct code being as follows:
/*jslint browser */
(function () {
"use strict";
function play() {
var player = document.getElementById("player");
player.volume = 1.0;
player.play();
}
function pause() {
var player = document.getElementById("player");
player.volume = 1.0;
if (player.paused) {
player.play();
} else {
player.pause();
}
}
document.getElementById("play").addEventListener("click", play, false);
document.getElementById("pause").addEventListener("click", pause, false);
}());
That is the more correct way of doing things.
From there are other little touches that can be applied, such as caching the player id variable so that it’s assigned at only the one place, or using a separate function to set the volume and apply certain actions to the player. But the time for using those types of techniques is better held off for now. The benefit of those techniques is of little value now, and of greater value later on when more code has been added.
This is how I’m using the codes, and they work fine set up this way.
<div style="width:0;" onclick="myObject=document.getElementById('myObj');
myObject.style.display='block'; this.style.display='none'">
<div style="margin:0 0 -24px;width: 260px;height:18px;border-radius:50px;background-image: linear-gradient( to right, #000000 83px,#0059dd 83px, #0059dd 86px, #000000 86px, #000000 174px, #0059dd 174px, #0059dd 177px, #000000 177px );border: 3px solid #0059dd;"></div>
<button style="display:block; border:none;cursor: pointer;background-color:transparent;width:266px;height:24px;" onclick="document.getElementById('player').volume=1.0;player.paused ? player.play() :player.pause()"></button>
</div>
<div id="myObj" style="display: none;">
<div style="margin:0 0 -24px;width: 260px;height:18px;border-radius:50px;background-image: linear-gradient( to right, #0ff 83px,#0059dd 83px, #0059dd 86px, #fff 86px, #fff 174px, #0059dd 174px, #0059dd 177px, #f0f 177px );border: 3px solid #0059dd;"></div>
<button style="display:block; border:none;cursor: pointer;background-color:transparent;width:266px;height:24px;" onclick="document.getElementById('player');player.paused ? player.play() : player.pause()"></button>
</div>
<audio id="player" preload="none" style="display:none;">
<source src='http://hi5.1980s.fm/;' type='audio/mpeg'/>
</audio>
[quote=“asasass, post:26, topic:230014, full:true”]
This is how I’m using the codes, and they work fine set up this way.
https://jsfiddle.net/44zr75pp/1/[/quote]
They work, but messing up your HTML CSS and JavaScript codes in with each other is failing to do it correctly.
If you want to learn more about how to do these things correctly, we have plenty of experts here that are willing to take the time to teach you how.
How am I messing them up if they work how I want them to work?
Are you interested in our opinions on why the are not set up correctly?
But they don’t work properly all the time - a lot of the time it is a jumbled mess because you have only catered for some situations and not all ways of viewing the page.
@asasass: is this some kind of school project where you’ve been told you have to code this way? (I know many courses tend to be out-of-date in their approach, but this one would be something of a record, I think.)
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.