That is completely inappropriate. Things just get too complex and difficult to understand when many types of things are being dealt with in the same place. The videoPlayer code should not have knowledge about the cover code.
It’s a programming idea called Separation of Concerns and is vital to achieve code that is easy to work with and understand.
You would say the same thing to this then.
Too complex.
code: https://jsfiddle.net/k4mu0svp/7/
function init(opts) {
load.js("https://www.youtube.com/player_api").then(function () {
YT.ready(function () {
addVideo(opts.video);
});
});
}
return {
init
};
}());
(function iife() {
"use strict";
function show(el) {
el.classList.remove("hide");
}
function hide(el) {
el.classList.add("hide");
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
const thewrap = cover.parentNode.querySelector(".wrap");
hide(cover);
videoPlayer.init({
video: document.querySelector(".video")
});
show(thewrap);
}
const cover = document.querySelector(".jacket");
cover.addEventListener("click", coverClickHandler);
}());
You have the init adding the video, which causes the video to start playing immediately. You said that you don’t want that to occur.
Yes, preventing the audio, and the video from playing behind the cover when first viewing the screen.
Yes I agree - the mixing of videoPlayer and cover code is inappropriate and shouldn’t be done.
Complexity is the enemy of the programmer, for it makes change much harder to occur. You know from your working with code that it hardly ever remains unchanged. The code needs to be able to change and adapt so that our needs can be achieved.
When that code is more difficult to change because it has too many things that can break, we tend to stop changing that code. Sometimes refactoring can occur to separate the code so that it’s then easier to change, but we shouldn’t wait for the code to be complex before doing that.
Keep things simpler from the beginning using ideas such as Separation of Concerns, and the code will continue to be easy to change.
Question, using your code, if I wanted to add a delay to the YouTube appearing on the screen after it is clicked, how would I do that?
code https://jsfiddle.net/ygv9woks/
function hide(el) {
el.classList.add("hide");
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
const thewrap = cover.parentNode.querySelector(".container");
hide(cover);
show(thewrap);
if (config.whenOpened instanceof Function) {
config.whenOpened();
}
}
const cover = document.querySelector(".jacket");
cover.addEventListener("click", coverClickHandler);
};
const videoPlayer = (function makeVideoPlayer() {
"use strict";
function onPlayerReady(event) {
const player = event.target;
player.setVolume(100); // percent
}
let hasShuffled = false;
function onPlayerStateChange(event) {
const player = event.target;
const shufflePlaylist = true;
if (!hasShuffled) {
player.setShuffle(shufflePlaylist);
player.playVideoAt(0);
hasShuffled = true;
}
}
function addVideo(video) {
const playlist = "M7lc1UVf-VE";
new YT.Player(video, {
width: 640,
height: 360,
host: "https://www.youtube-nocookie.com",
playerVars: {
autoplay: 0,
controls: 1,
loop: 1,
rel: 0,
iv_load_policy: 3,
cc_load_policy: 0,
fs: 0,
disablekb: 1,
playlist
},
events: {
"onReady": onPlayerReady,
"onStateChange": onPlayerStateChange
}
});
}
function init(opts) {
const curtainsConfig = {
whenOpened: function () {
addVideo(opts.video);
}
};
load.js("https://www.youtube.com/player_api").then(function () {
YT.ready(function () {
makeCurtains(curtainsConfig);
});
});
}
return {
init
};
}());
videoPlayer.init({
video: document.querySelector(".video")
});
How much of a delay? Until the curtains have finished opening?
Where I would be able to set the number.
1, 2, 3, 4, 5 ,6 etc. seconds.
That can be added to the makeCurtains config parameter. As it is whenOpened that we want to delay, it can be called delayOpen.
Usually time is in either seconds or milliseconds. Because the seconds tend to be such a low number, it’s preferred to use milliseconds instead. That way the milliseconds value can be directly passed to the setTimeout function too without needing to do math.
const curtainsConfig = {
whenOpened: function () {
addVideo(opts.video);
},
delayOpen: 3000
};
In the coverClickHandler function we can move the whenOpened code out to a separate function, so that it can later on be called from setTimeout.
function whenOpenedHandler(config) {
if (config.whenOpened instanceof Function) {
config.whenOpened();
}
}
function coverClickHandler(evt) {
...
// if (config.whenOpened instanceof Function) {
// config.whenOpened();
// }
whenOpenedHandler(config);
}
And now that we’ve simplified that part of things, the whenOpenedHandler can be put into a setTimeout call.
To protect against the possibility of delayOpen being undefined, we use the || operator to give it a default value of 0 if no delayOpen value is given.
// whenOpenedHandler(config);
setTimeout(function () {
whenOpenedHandler(config);
}, config.delayOpen || 0);
And it’s all done. https://jsfiddle.net/gebq492x/
You only now need to adjust the delayOpen value in the curtainsConfig object, which is in the init section.
const curtainsConfig = {
whenOpened: function () {
addVideo(opts.video);
},
delayOpen: 5000
};
About the only other config-related thing to do from there is to move the configurable information up to the top of the code, so that it’s easy to find when you later on come back to the code.
How is that done?
It would all go into it’s own function?
It would all into here?
const makeCurtains = function iife(config) {
"use strict";
function show(el) {
el.classList.remove("hide");
document.querySelector(".curtain").classList.add("slide");
}
function hide(el) {
el.classList.add("hide");
}
function whenOpenedHandler(config) {
if (config.whenOpened instanceof Function) {
config.whenOpened();
}
}
All of this config?
It would then be moved to the top.
function init(opts) {
const curtainsConfig = {
whenOpened: function () {
addVideo(opts.video);
},
delayOpen: 5000
};
load.js("https://www.youtube.com/player_api").then(function () {
YT.ready(function () {
makeCurtains(curtainsConfig);
});
});
}
return {
init
};
}());
The curtainsConfig is pretty easy - just move the delayOpen part to the top of the code.
const config = {
curtains: {
delayOpen: 3000
}
};
const load = (function () {
...
and in the init function, we refer to that config.curtains object:
const curtainsConfig = {
whenOpened: function () {
addVideo(opts.video);
},
delayOpen: config.curtains.delayOpen
};
That’s the cover config all dealt with. https://jsfiddle.net/xag9m8st/
More difficult is the following player config:
function addVideo(video) {
const playlist = "M7lc1UVf-VE";
new YT.Player(video, {
width: 640,
height: 360,
host: "https://www.youtube-nocookie.com",
playerVars: {
autoplay: 0,
controls: 1,
loop: 1,
rel: 0,
iv_load_policy: 3,
cc_load_policy: 0,
fs: 0,
disablekb: 1,
playlist
},
events: {
"onReady": onPlayerReady,
"onStateChange": onPlayerStateChange
}
});
First, the playlist is moved into playerVars.
// const playlist = "M7lc1UVf-VE";
playerVars: {
playlist: "M7lc1UVf-VE",
autoplay: 0,
...
disablekb: 1
// disablekb: 1,
// playlist
},
We can now extract the player config out to a separate playerConfig object:
const playerConfig = {
width: 640,
...
};
new YT.Player(video, config);
We don’t want all of that playerConfig up at the top of the page. The events part should remain where it is. How we deal with that is to add a player section to the config, and copy the player config information there.
const config = {
curtains: {
...
},
player: {
width: 640,
height: 360,
host: "https://www.youtube-nocookie.com",
playerVars: {
playlist: "M7lc1UVf-VE",
autoplay: 0,
controls: 1,
loop: 1,
rel: 0,
iv_load_policy: 3,
cc_load_policy: 0,
fs: 0,
disablekb: 1
}
}
};
Further down the code in the addVideo function, we refer to parts of that config.player object:
const playerConfig = {
width: config.player.width,
height: config.player.height,
host: config.player.host,
playerVars: config.player.playerVars,
events: {
"onReady": onPlayerReady,
"onStateChange": onPlayerStateChange
}
};
And that’s the player config all dealt with. https://jsfiddle.net/xag9m8st/1/
The configurable information is now all at the top of the code, making it easier to access.
How do you add more than 1 video to it?
This way didn’t work.
playlist: "M7lc1UVf-VE, mnfmQe8Mv1g",
How it worked before:
const playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g";
Got it.
playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
And if I want to remove the delay from the code, it is able to be removed easily.
Like I did here:
code https://jsfiddle.net/72pnouxa/1/
const config = {
player: {
width: 640,
height: 360,
host: "https://www.youtube-nocookie.com",
playerVars: {
playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
autoplay: 0,
controls: 1,
loop: 1,
rel: 0,
iv_load_policy: 3,
cc_load_policy: 0,
fs: 0,
disablekb: 1
}
}
};
const load = (function() {
"use strict";
function _load(tag) {
return function(url) {
return new Promise(function(resolve) {
const element = document.createElement(tag);
const parent = "body";
const attr = "src";
element.onload = function() {
resolve(url);
};
element[attr] = url;
document[parent].appendChild(element);
});
};
}
return {
js: _load("script")
};
}());
const makeCurtains = function iife(config) {
"use strict";
function show(el) {
el.classList.remove("hide");
document.querySelector(".curtain").classList.add("slide");
}
function hide(el) {
el.classList.add("hide");
}
function whenOpenedHandler(config) {
if (config.whenOpened instanceof Function) {
config.whenOpened();
}
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
const thewrap = cover.parentNode.querySelector(".container");
hide(cover);
show(thewrap); {
whenOpenedHandler(config);
}
}
const cover = document.querySelector(".jacket");
cover.addEventListener("click", coverClickHandler);
};
const videoPlayer = (function makeVideoPlayer() {
"use strict";
function onPlayerReady(event) {
const player = event.target;
player.setVolume(100); // percent
}
let hasShuffled = false;
function onPlayerStateChange(event) {
const player = event.target;
const shufflePlaylist = true;
if (!hasShuffled) {
player.setShuffle(shufflePlaylist);
player.playVideoAt(0);
hasShuffled = true;
}
}
function addVideo(video) {
const playerConfig = {
width: config.player.width,
height: config.player.height,
host: config.player.host,
playerVars: config.player.playerVars,
events: {
"onReady": onPlayerReady,
"onStateChange": onPlayerStateChange
}
};
new YT.Player(video, playerConfig);
}
function init(opts) {
const curtainsConfig = {
whenOpened: function() {
addVideo(opts.video);
}
};
load.js("https://www.youtube.com/player_api").then(function() {
YT.ready(function() {
makeCurtains(curtainsConfig);
});
});
}
return {
init
};
}());
videoPlayer.init({
video: document.querySelector(".video")
});
To remove the delay just set the delayOpen value to 0.
I thought this was supposed to stay at the top of the page, it’s not?
All the other javascript would go underneath it.
const load = (function () {
"use strict";
function _load(tag) {
return function (url) {
return new Promise(function (resolve) {
const element = document.createElement(tag);
const parent = "body";
const attr = "src";
element.onload = function () {
resolve(url);
};
element[attr] = url;
document[parent].appendChild(element);
});
};
}
return {
js: _load("script")
};
}());
The config information is more important to be at the top. That is what tends to get adjusted more often than not, which is why it’s become such a standard to put config information in the most easily accessible area, at the top of the code.
When looking at the script at https://jsfiddle.net/xag9m8st/1/ it can now be summarised as:
There are two issues that I see.
Issue 1: The load code should go below makeCurtains, so that the load code is more closely associated with the videoPlayer code.
Issue 2: The videoPlayer code has its own separate init, inside of which makeCurtains is dealt with. The videoPlayer should have no knowledge of makeCurtains. That information about makeCurtains should be passed in to the videoPlayer init instead.
This is as easy as moving the load function down, so that it’s between the makeCurtains and videoPlayer code.
// const load = (function() {
// ...
// }());
const makeCurtains = function iife(config) {
...
};
const load = (function() {
...
}());
const videoPlayer = (function makeVideoPlayer() {
In fact, that load code isn’t even needed. We can move the player_api to where it properly belongs in the Resources section. That way, https://www.youtube.com/player_api is loaded before the script runs.
We can now completely remove the load code, and the parts that relied on it.
// const load = (function() {
// ...
// }());
...
// load.js("https://www.youtube.com/player_api").then(function() {
// YT.ready(function() {
makeCurtains(curtainsConfig);
// });
// });
We can move that init section out of the videoPlayer, so that it becomes a generic init at the end of the code instead.
The only problem is that addVideo is not available, so we’ll add it to the videoPlayer return.
// function init(opts) {
// ...
// }
return {
// init
addVideo
};
}());
function init(videoEl) {
const curtainsConfig = {
whenOpened: function() {
videoPlayer.addVideo(videoEl);
},
delayOpen: config.curtains.delayOpen
};
makeCurtains(curtainsConfig);
}
init(document.querySelector(".video"));
The code structure is now:
which is a much better structure to use. The only place that the curtains and player interact now is in the init. Not in makeCurtains, not in videoPlayer, but in init where they belong.
Sorry about that, I had the wrong fiddle link, fixed now.
I wanted to see and know how to do something.
How would I add the setTimeout(function()
To this code, by itself?
code https://jsfiddle.net/hmueokar/
(function manageCover() {
"use strict";
function show(el) {
el.classList.remove("hide");
document.querySelector(".curtain").classList.add("slide");
}
function hide(el) {
el.classList.add("hide");
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
const thewrap = cover.parentNode.querySelector(".container");
hide(cover);
show(thewrap);
}
const cover = document.querySelector(".jacket");
cover.addEventListener("click", coverClickHandler);
}());
const videoPlayer = (function makeVideoPlayer() {
"use strict";
function onPlayerReady(event) {
const player = event.target;
player.setVolume(100); // percent
}
let hasShuffled = false;
function onPlayerStateChange(event) {
const player = event.target;
const shufflePlaylist = true;
if (!hasShuffled) {
player.setShuffle(shufflePlaylist);
player.playVideoAt(0);
hasShuffled = true;
}
}
function addVideo(video) {
const playlist = "M7lc1UVf-VE";
new YT.Player(video, {
width: 640,
height: 360,
host: "https://www.youtube-nocookie.com",
playerVars: {
autoplay: 0,
controls: 1,
loop: 1,
rel: 0,
iv_load_policy: 3,
cc_load_policy: 0,
fs: 0,
disablekb: 1,
playlist
},
events: {
"onReady": onPlayerReady,
"onStateChange": onPlayerStateChange
}
});
}
function init(opts) {
load.js("https://www.youtube.com/player_api").then(function () {
YT.ready(function () {
addVideo(opts.video);
});
});
}
return {
init
};
}());
(function iife() {
"use strict";
function show(el) {
el.classList.remove("hide");
}
function initPlayer(wrapper) {
videoPlayer.init({
video: wrapper.querySelector(".video")
});
}
function coverClickHandler(evt) {
const wrapper = evt.currentTarget.parentElement;
show(wrapper);
initPlayer(wrapper);
}
const cover = document.querySelector(".jacket");
cover.addEventListener("click", coverClickHandler);
}());