Let’s run the code through JSLint to find easy ways to make the code better.
This function needs a “use strict” pragma.
The “use strict” part cannot be a comment, it doesn’t work as a comment, and only achieves something when it is a text string.
(function iife() {
/* "use strict";*/ // This does nothing when it's as a comment.
"use strict"; // This string improves how the browser handles the code.
Undeclared ‘document’.
Tell JSLint that you expect that the code be run in a browser.
/*jslint browser */
...
Expected ‘;’ and instead saw ‘,’.
Define values as separate statements, to improve your ability to refactor and improve the code.
// var div, n, v = document.getElementsByClassName("youtube");
var div;
var n;
var v = document.getElementsByClassName("youtube");
Unexpected ‘for’.
For loops tend to make it more difficult to understand the code. In this case v is a nodeList and we are doing something with each v element, so we can use the forEach method instead.
// for (n = 0; n < v.length; n++) {
v.forEach(function (video) {
div = document.createElement("div");
// div.setAttribute("data-id", v[n].dataset.id);
div.setAttribute("data-id", video.dataset.id);
// div.innerHTML = thumb(v[n].dataset.id);
div.innerHTML = thumb(video.dataset.id);
div.onclick = iframe;
// v[n].appendChild(div);
video.appendChild(div);
// }
});
‘thumb’ is out of scope.
Move the thumb code up above. It’s not vital when the code is a function declaration, but it does make a difference when const/let/var are used to declare functions.
function thumb(id) {
...
}
document.addEventListener("DOMContentLoaded", function() {
...
});
// function thumb(id) {
// ...
// }
Redefinition of ‘thumb’ from line 4.
Don’t repeat function names and variable names, as it leads to confusion that’s best fixed up now. It helps if the function has the more expressive name using a verb+noun format, so we can call this createThumbnail instead.
// function thumb(id) {
function createThumbnail(id) {
...
}
...
// div.innerHTML = thumb(video.dataset.id);
div.innerHTML = createThumbnail(video.dataset.id);
Use double quotes, not single quotes.
Using double quotes is best as we’re more likely to use apostrophes in text, resulting in double quotes around them. Also, we should not be using HTML code within JavaScript, so this is a handy way to find such abuses.
I’ll also separate those multiple var statements while we’re at it too.
// var thumb = '<img src="https://i.imgur.com/AJDZEOX.jpg">',
var thumb = "<img src='https://i.imgur.com/AJDZEOX.jpg'>";
// play = '<div class="play"></div>';
var play = "<div class='play'></div>";
‘iframe’ is out of scope.
Move that function up above where it’s used too.
function createThumbnail(id) {
...
}
function iframe() {
...
}
document.addEventListener("DOMContentLoaded", function() {
...
});
// function iframe() {
// ...
// }
Redefinition of ‘iframe’ from line 9.
Give the iframe function a more expressive name.
// function iframe() {
function replaceThumbWithIframe() {
Unexpected ‘this’.
The this keyword can become really confusing about what it refers to, so using a more expressive technique becomes really handy.
// function replaceThumbWithIframe() {
function replaceThumbWithIframe(evt) {
const thumb = evt.currentTarget;
...
// iframe.setAttribute("src", embed.replace("ID", this.dataset.id));
iframe.setAttribute("src", embed.replace("ID", thumb.dataset.id));
...
// this.parentNode.replaceChild(iframe, this);
thumb.parentNode.replaceChild(iframe, thumb);
}
Expected one space between ‘function’ and ‘(’.
It’s better to use named functions instead of using anonymous functions
// document.addEventListener("DOMContentLoaded", function() {
document.addEventListener("DOMContentLoaded", function init() {
But that’s not going to work because it’s too late for the DOMContentLoaded event. The onload event and even nowrap at the bottom of the body, ensures that the dom content is already loaded.
So we can turn that into just an ordinary init function and run that instead.
// document.addEventListener("DOMContentLoaded", function init() {
function init() {
...
// });
}
init();
Unused ‘n’.
After our earlier loop improvement the n index variable is no longer used, and can be removed.
// var n;
Using const instead of var
You were wanting to use const instead of var, which I fully support too, so we can replace all of the var statements with const instead.
Expected ‘=’ and instead saw ‘;’.
A const variable cannot be unassigned.
const div;
const v = document.getElementsByClassName("youtube");
v.forEach(function (video) {
div = document.createElement("div");
We should move that const div into the forEach function instead.
// const div;
const v = document.getElementsByClassName("youtube");
v.forEach(function (video) {
const div = document.createElement("div");
We can now copy this updated code from JSLint back to the JSFiddle code.
Uncaught TypeError: v.forEach is not a function
When trying to run the code, we’re told that v.forEach is not a function. That’s because v is undefined.
Why is v undefined?
const v = document.getElementsByClassName("youtube");
It seems that getElementsByClassName doesn’t give us a nodeList that we can easily iterate over. That’s alright, for we have querySelectorAll instead.
// const v = document.getElementsByClassName("youtube");
const v = document.querySelectorAll(".youtube");
Tidying up the thumbnail
I’m noticing that you want to use a specific thumbnail for this particular example:
function createThumbnail(id) {
const thumb = "<img src='https://i.imgur.com/AJDZEOX.jpg'>";
const play = "<div class='play'></div>";
return thumb.replace("ID", id) + play;
}
Instead of changing the createThumbnail code, it’s better to pass in that custom imagename instead:
function createThumbnail(id) {
// const thumb = "<img src='https://i.imgur.com/ID.jpg'>";
const play = "<div class='play'></div>";
return thumb.replace("ID", id) + play;
}
...
// div.innerHTML = createThumbnail(video.dataset.id);
div.innerHTML = createThumbnail("AJDZEOX");
And that’s the code there appropriately updated. https://jsfiddle.net/pmw57/zna6h7r9/
