Adding a pauseVideo in the shuffle command helps to alleviate that.
player.setShuffle(true);
player.playVideoAt(0);
player.pauseVideo();
}
The pause is needed there because we are using playVideoAt to reset to the first video in the newly shuffled list.
On the single player code, onPlayerStateChange can be removed from the code.
Which is an improvement because it is code that is no longer needed or being used.
const videoPlayer = (function makeVideoPlayer() {
"use strict";
let player = null;
const tag = document.createElement("script");
tag.src = "https://www.youtube.com/iframe_api";
const firstScriptTag = document.getElementsByTagName("script")[0];
firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);
function shufflePlaylist(player) {
player.setShuffle(true);
player.playVideoAt(0);
player.pauseVideo();
}
function onPlayerReady(event) {
const player = event.target;
player.setVolume(100); // percent
shufflePlaylist(player);
}
function addPlayer(video) {
const playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g";
const config = {
height: 360,
host: "https://www.youtube-nocookie.com",
width: 640
};
config.playerVars = {
autoplay: 0,
cc_load_policy: 0,
controls: 1,
disablekb: 1,
fs: 0,
iv_load_policy: 3,
loop: 1,
playlist,
rel: 0
};
config.events = {
"onReady": onPlayerReady
};
player = new YT.Player(video, config);
}
But what if the aim is to have only one set of code that can work in all situations? Then you are not improving things but are creating another divergence instead.
If it is only being used with a single player, what situation would call for onPlayerStateChange to be needed?
I think that we have learned that you and me have different objectives.
You seem to want different code that must change for every different type of situation
Whereas I want one set of code that doesn’t need to change for different situations.
Wouldn’t something only be added to code if the situation calls for it, and if something is not being used, it can be left out.
It’s best for code to be generic enough so that it can handle a wide variety of situations. That is why we pass parameters to functions, so that the functions don’t need to be rewritten when we need something different done.
That is also why code libraries are configurable from the outside, so that we don’t have to go in and change things internally.
Does new RandomNumber only work with onPlayerStateChange?
Meaning, it’s not meant to or supposed to work with onPlayerReady.
function newRandomNumber(min, max) {
return Math.floor(Math.random() * max) + min;
}
works here
https://jsitor.com/hi-kYiM7DA
function onPlayerStateChange(event) {
player = event.target;
player.setShuffle(true);
}
function newRandomNumber(min, max) {
return Math.floor(Math.random() * max) + min;
}
Doesn’t work here:
https://jsitor.com/E8JILS8mzZ
function shufflePlaylist(player) {
player.setShuffle(true);
player.playVideoAt(0);
player.pauseVideo();
}
function onPlayerReady(event) {
const player = event.target;
player.setVolume(100);
shufflePlaylist(player);
}
function newRandomNumber(min, max) {
return Math.floor(Math.random() * max) + min;
}
I’ll come back to that. I’m working on improving the fake onPlayerReady event handling stuff, so that it uses proper events to do things for us.
In the videoPlayer code, I now want to replace the fake onPlayerReady code.
function onPlayerReady(event) {
...
if (player.onPlayerReady) {
player.onPlayerReady(player);
}
}
...
function addPlayer(video, settings) {
const onPlayerReady = settings.onPlayerReady;
delete settings.onPlayerReady;
...
const player = new YT.Player(video, playerVars);
player.onPlayerReady = onPlayerReady;
...
}
...
const player = (function makePlayer() {
...
const onReady = playerReadyWrapper(cover, events.onPlayerReady);
videoPlayer.initPlayer(videoWrapper, settings, onReady);
...
}
That will be improved by using some real onPlayerReady event code instead.
A more proper event-based way of doing that is as follows. This example is from the MDN Creating and triggering events documentation page.
const event = new Event('build');
// Listen for the event.
elem.addEventListener('build', function (e) { /* ... */ }, false);
// Dispatch the event.
elem.dispatchEvent(event);
It is also getting a little confusing having two different onPlayerReady references. One for the youtube code to use and one for our own code. We can rename ours to be afterPlayerReady instead.
In our code, we have an events object to store any custom events that we want to use.
const videoPlayer = (function makeVideoPlayer() {
"use strict";
const players = [];
const events = {
afterPlayerReady: new Event("afterPlayerReady")
};
}
Where do we add the event listener? It can’t be on the player object because it’s only elements that support events. It can’t be in addPlayer on the video element because youtube destroys that when it creates the iframe. Can it be on the iframe? We can certainly try that.
function getIframe(player) {
return player.h;
}
...
function addEvents(player, settings) {
const iframe = getIframe(player);
iframe.addEventListener("afterPlayerReady", settings.afterPlayerReady);
}
...
function addPlayer(video, settings) {
...
const player = new YT.Player(video, playerVars);
addEvents(player, settings);
...
}
And we can dispatch the event from the end of the onPlayerReady handler.
function onPlayerReady(event) {
const player = event.target;
const iframe = getIframe(player);
...
iframe.dispatchEvent(events.afterPlayerReady);
}
We can then setup the event when we add a player:
const eventWrapper = playerReadyWrapper(cover, events.afterPlayerReady);
settings.afterPlayerReady = eventWrapper;
videoPlayer.initPlayer(videoWrapper, settings);
Replacing onAddPlayer with afterAddPlayer helps to coordinate the distinction that things with an “on” prefix are in code outside of our control, whereas those with a prefix of “after” are within our control.
The updated code is at https://jsitor.com/1xqkXKIrsv and I’ll investigate that random number thing next.
The newRandomNumber function is used with playerVars, to set a different starting index.
There’s no good reason to do that when the playlist is also shuffled.
Actually, I think both these things work with each other.
After every song, all the songs get scrambled into a new list order.
function onPlayerStateChange(event) {
player = event.target;
player.setShuffle(true);
}
function newRandomNumber(min, max) {
return Math.floor(Math.random() * max) + min;
}
Everything in onPlayerStateChange is happening on every single interaction or change to the player. When it’s buffering, when you hit pause, when you hit play, when the player comes to an end, or when it starts. Everything.
I’m going to work on some tests, making them easy to achieve and easy to ignore.
Two library items are added to jsitor, one for mocha which is the testing framework, and one for chai which performs the assertions.
We can tell the test to show the report as a list in the console:
/* TESTS */
mocha.setup({
checkLeaks: true,
reporter: "list",
ui: "bdd"
});
const expect = chai.expect;
describe("Simple test", function () {
it("can pass a test", function () {
expect(true).to.equal(true);
});
it("can fail a test", function () {
expect(false).to.equal(true);
});
});
mocha.run();
That way the browser area of jsitor is not affected, and in the console we get a list of what passes and fails.
Now that things are confirmed as working, I prefer to replace the list reporter with just the dot reporter, as that stays nice and quiet on passing tests and only gives detail on failing tests.
mocha.setup({
checkLeaks: true,
reporter: "dot",
ui: "bdd"
});
From here on out it can just be a simple matter of adding tests for new or untested behaviour.
I’ll start nice and small, with the spinner code. We only need to check that it adds or removes a classname.
describe("Spinner", function () {
const div = document.createElement("div");
it("toggles on when the classname doesn't exist", function () {
div.classList.remove("lds-dual-ring");
spinner.toggleDualRing(div);
expect(div.classList.contains("lds-dual-ring")).to.equal(true);
});
it("toggles off when the classname already exists", function () {
div.classList.add("lds-dual-ring");
spinner.toggleDualRing(div);
expect(div.classList.contains("lds-dual-ring")).to.equal(false);
});
});
The rest of it is about as simple. Pick small behaviours that are expected to remain consistent, and ensure that they are working as expected. That way later on when we break something that isn’t immediately visible, we get instant feedback about it.
When testing manageCover, I want to know if clicking a cover plays a video. I won’t actually check that a video plays yet, I can worry about that when testing videoPlayer. But with manageCover, I just want to know if it calls the videoPlayer.play method.
Currently I have a temporary solution where I replace videoPlayer.play with my own function that sets a variable called playWasCalled. That means storing the original play method aside in a cache until I’m done with testing.
describe("Manage cover", function () {
const cache = {};
let playWasCalled;
before(function replaceVideoPlayer() {
cache.play = videoPlayer.play;
videoPlayer.play = function playVideo() {
playWasCalled = true;
};
});
beforeEach(function () {
playWasCalled = false;
...
});
after(function restoreVideoPlayer() {
videoPlayer.play = cache.play;
});
..
it("when clicked plays the video", function () {
manageCover.init(cover);
cover.click();
expect(playWasCalled).to.equal(true);
});
});
That’s effective, but clumsy.
A much better way to do that us by using chai-spies.
I’ve told jsitor to use it from the following CDN https://cdn.jsdelivr.net/npm/chai-spies@1.0.0/chai-spies.min.js
I can now replace all of the above code with the following much simplified solution, where I just spy on a fake player.playVideo method and check if that has been called.
let wrapper;
let player;
beforeEach(function () {
...
player = {};
chai.spy.on(player, "playVideo");
wrapper.player = player;
...
});
...
it("when clicked plays the video", function () {
manageCover.init(cover);
expect(player.playVideo).to.not.have.been.called();
cover.click();
expect(player.playVideo).to.have.been.called();
});
That’s a much better solution than the caching one.
I can now move on to putting together some tests for the videoPlayer code.
Are all the videos supposed to play when they are clicked on?
They don’t turn off when another is clicked on.
Good - I can use a test to help with that. First, I’ll write a test the the expected behaviour that we want, then I’ll update the code so that it achieves that behaviour.
That way we’ll have that test in place for if other changes affect that as well.
Dammit, the code is really tough to access via testing, which is an unfortunate side-effect of writing it without using tests.
One approach to dealing with that is to rewrite the code but use tests to define each new behaviour that is wanted. Another approach is to gradually add tests for more and more things, adjusting things while you go so that things are easier to test.
I think that I’ll go for the second approach there.
In the meantime my earlier concerns about the i.j from player.i.j.playerVars in post #78 being a problem occurred. Youtube changed it to i.h instead. I now have a different approach that is more stable and works no matter whether it is j or h or anything else.
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.