Adding Const defaultPlayerVars =

How would I be able to add this in:

  function initPlayer(videoWrapper, settings) {
    const video = videoWrapper.querySelector(".video");
    const defaultPlayerVars = {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      height: 207,
      iv_load_policy: 3,
      rel: 0,
      width: 277
    };
    const playerVars = Object.assign({}, defaultPlayerVars, settings);

    function paramInSettings(obj, param) {
      if (settings[param] !== undefined) {
        obj[param] = settings[param];
        delete settings[param];
      }
      return settings;
    }

    const settingsParams = ["width", "height", "videoid", "host"];
    const playerSettings = settingsParams.reduce(paramInSettings, {});
    playerSettings.playerVars = playerVars;
    const player = videoPlayer.addPlayer(video, playerSettings);
    videoWrapper.player = player;
  }

In place of this?
https://jsfiddle.net/y1vm5teq/

    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");
        opts.width = opts.width || 198;
        opts.height = opts.height || 198;
        opts.autoplay = 0;
        opts.controls = 1;
        opts.rel = 0;
        opts.enablejsapi = 1;
        opts.iv_load_policy = 3;
        opts.fs = 0;
        opts.disablekb = 1;

        function paramInOpts(settings, param) {
            if (opts[param] !== undefined) {
                settings[param] = opts[param];
            }
            return settings;
        }
        const settingsParams = ["width", "height", "videoid", "host"];
        const settings = settingsParams.reduce(paramInOpts, {});
        settings.playerVars = opts;
        videoPlayer.addPlayer(video, settings);

    }

That being the first change I want to make before deciding what I want to do after that.

I think that it’s going to help to clearly define the terms.

  • playerVars is the most clear, that defines several types of behaviour for the player.
  • Youtube says about the second parameter given to add a player: “The second parameter is an object that specifies player options.” so calling that object options or playerOptions makes good sense.
  • playerVars is a parameter of that options object.
  • The settings object combines both playerVars and options parameters for the sake of convenience. That combination really needs to be separated before using them.

When those terms are being used for different types of things, that is where confusion easily occurs. That confusion should be removed at the earliest convenience.

With that out of the way, how I would add in that other initPlayer function is by renaming the old one to oldInitPlayer and having the new one in there too as initPlayer.

That way getting things to work with the new initPlayer function can then occur in relative safety, and when things work with it the oldInitPlayer can then be removed.

If no settings are given to initPlayer then we don’t want that to change from the default settings. The function should still be able to run even with no settings though, so we can tell the function to use an empty object by default when no settings are given.

function initPlayer(videoWrapper, settings = {}) {

Because we want those settings to only be for the player options and playerVars, we need to remove other things that don’t belong there after using them.

    const cover = document.querySelector(opts.target);
    delete opts.target;
    cover.addEventListener("click", coverClickHandler);

Currently the loadPlayer function calls its parameter opts. Those are not just options for the video player though. They are also configuration information for the cover. That means that we should call the loadPlayer parameter something other than opts, for which cover is a good choice.

That way information flows like this:

config => cover & settings
settings => options & playerVars

When renaming opts to config, I’ll keep the oldInitPlayer working by passing in the config, that it receives as opts.

function loadPlayer(config) {
    ...
    function oldInitPlayer(wrapper, opts) {
        ...
    }

    function coverClickHandler(evt) {
        ...
        show(wrapper);
        oldInitPlayer(wrapper, config);
    }

    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);
}

Now is a good time to switch things over from oldInitPlayer to the new initPlayer.

        show(wrapper);
        initPlayer(wrapper, config);

In the initPlayer function is now a good time to rename variables to options so that they fall in line with the above naming guidelines.

        function paramInOptions(options, param) {
            if (settings[param] !== undefined) {
                options[param] = settings[param];
                delete settings[param];
            }
            return options;
        }
...
        const optionParams = ["width", "height", "videoid", "host"];
        const playerOptions = optionParams.reduce(paramInOptions, {});
...
        playerOptions.playerVars = playerVars;
        const player = videoPlayer.addPlayer(video, playerOptions);

I console.log some variables to help ensure that they are behaving themself. The playerVars object is misbehaving.

{
  autoplay: 0
  controls: 1
  disablekb: 1
  enablejsapi: 1
  fs: 0
  height: 207
  iv_load_policy: 3
  rel: 0
  width: 277
}

The width and height are inappropriately defined in the playerVar defaults. Those should be separately defined as playerOption defaults.

        const defaultPlayerOptions = {
            height: 207,
            width: 277
        };

That way, once we’ve got the preferred options for the player, we can combine them together with the default options. If the preferred ones aren’t specified in the config then the default ones will take place instead.

        const optionParams = ["width", "height", "videoid", "host"];
        const preferredOptions = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, defaultPlayerOptions, preferredOptions);

The width and height are still inappropriately in playerVars. We need to create that after the player options, so that the remainder can then be assigned to playerVars. That should be as easy as moving the playerVars code down below the options code.

        const playerOptions = optionParams.reduce(paramInOptions, {});
        const playerVars = Object.assign({}, defaultPlayerVars, settings);

It becomes clear now that the job of initPlayer is to crate a properly structured set of player options, and use that to add a player. That information is useful enough and not easily represented by the code, that we should add it as a comment to the function.

    function initPlayer(videoWrapper, settings = {}) {
        // Create a properly structured player options object
        // and use it to add a player.

We can now get rid of oldInitPlayer, and the updated code is at https://jsfiddle.net/bg731cwo/

1 Like

Inspired by that comment, here is a separate function that creates the player options:

    function createPlayerOptions(settings) {
        const defaultOptions = {
            height: 207,
            width: 277
        };
        const defaultPlayerVars = {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3,
            rel: 0
        };

        function paramInOptions(options, param) {
            if (settings[param] !== undefined) {
                options[param] = settings[param];
                delete settings[param];
            }
            return options;
        }

        const optionParams = ["width", "height", "videoid", "host"];
        const options = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, defaultOptions, options);
        const playerVars = Object.assign({}, defaultPlayerVars, settings);

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }

We can now use that createPlayerOptions function and remove the previously needed comments.

    function initPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(settings);
        const player = videoPlayer.addPlayer(video, playerOptions);
        videoWrapper.player = player;
    }

The updated code is found at https://jsfiddle.net/w90apn74/

1 Like

As another refinement, it’s not appropriate for default parameters to be specified in the createPlayerOptions function. Let’s move them out.

    function createPlayerOptions(options, settings) {
        function paramInOptions(opts, param) {
            if (settings[param] !== undefined) {
                opts[param] = settings[param];
                delete settings[param];
            }
            return opts;
        }

        const optionParams = ["width", "height", "videoid", "host"];
        const preferred = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, options, preferred);
        const playerVars = Object.assign({}, options.playerVars, settings);

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }

    function initPlayer(videoWrapper, options, settings = {}) {
        ...
        const playerOptions = createPlayerOptions(options, settings);
        ...
    }

The clickHandler is somewhere further out to specify defaults.

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        const options = {
            height: 207,
            width: 277
        };
        options.playerVars = {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3,
            rel: 0
        };
        initPlayer(wrapper, options, config);
    }

But even then the clickHandler isn’t the appropriate place to specify the defaults.

What makes better sense is to initialise the player defaults when initializing the cover.

    const defaults = {
        height: 207,
        width: 277
    };
    defaults.playerVars = {
        autoplay: 0,
        controls: 1,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
    };
    initPlayerDefaults(defaults);
    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);

We can then feed those defaults into where they need to be.

function loadPlayer(config) {
    const playerDefaults = {};
...
    function coverClickHandler(evt) {
        ...
        initPlayer(wrapper, config);
    }
...
    function createPlayerOptions(settings) {
        ...
        const playerOptions = Object.assign({}, playerDefaults, preferred);
        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }
...
    function initPlayerDefaults(defaults) {
        Object.assign(playerDefaults, defaults);
    }

    function initPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(settings);
        const player = videoPlayer.addPlayer(video, playerOptions);
        ...
    }

Doing things that way is a lot more stable and reliable.

1 Like

How would these be fixed in the code?

I was able to get it down to 2. and wasn’t able to adjust that one.

Also,

Adding a playlist:
playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"

works here:
https://jsfiddle.net/w90apn74/

But not here: in the updated code
https://jsfiddle.net/ko71m5gj/2/

I added a playlist here: you can see nothing appears
https://jsfiddle.net/u9royk8m/

    loadPlayer({
        height: 207,
        target: ".jacket-left",
        playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
        width: 277
    });

Can I take a look at the code that causes the issue?

This is the code I added a playlist to:

It does not work here.
Post#4

https://jsfiddle.net/u9royk8m/

  loadPlayer({
        height: 207,
        target: ".jacket-left",
        playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
        width: 277
    });

The last working version where the playlist worked:

Post#3

https://jsfiddle.net/xmpzbsqt/

Post#2

https://jsfiddle.net/7tm6qcu2/1/

In both post 3, and 2, something needs to be adjusted in here:

And those changes would be carried over to post 4.

When the cover is clicked on the playlist player, the video should be in the off state.

    function shufflePlaylist(player) {
        player.setShuffle(true);
        player.playVideoAt(0);
        player.pauseVideo();
    }

    function onPlayerReady(event) {
        player = event.target;
        player.setVolume(100); // percent
        shufflePlaylist(player);
    }

The single player on the right is in the off state when it is clicked on.

The playlist player on the left is in the on state when it is clicked on when it should be off, looking the same as the right player.

I tried moving stuff around but I wasn’t able to figure it out.

Playlist works when added: Post#3

playlist: “0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g”,

https://jsfiddle.net/xmpzbsqt/

function loadPlayer(config) {
    function show(el) {
        el.classList.remove("hide");
    }

    function createPlayerOptions(settings) {
        const defaultOptions = {
            height: 207,
            width: 277
        };
        const defaultPlayerVars = {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3,
            rel: 0
        };

        function paramInOptions(options, param) {
            if (settings[param] !== undefined) {
                options[param] = settings[param];
                delete settings[param];
            }
            return options;
        }

        const optionParams = ["width", "height", "videoid", "host"];
        const options = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, defaultOptions, options);
        const playerVars = Object.assign({}, defaultPlayerVars, settings);

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }

    function initPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(settings);
        const player = videoPlayer.addPlayer(video, playerOptions);
        videoWrapper.player = player;
    }

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        initPlayer(wrapper, config);
    }
    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);
}

Playlist doesn’t work when added. Post#4

playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",

https://jsfiddle.net/u9royk8m/

function loadPlayer(config) {
    const playerDefaults = {};

    function show(el) {
        el.classList.remove("hide");
    }

    function createPlayerOptions(settings) {
        function paramInOptions(opts, param) {
            if (settings[param] !== undefined) {
                opts[param] = settings[param];
                delete settings[param];
            }
            return opts;
        }

        const optionParams = ["width", "height", "videoid", "host"];
        const preferred = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, playerDefaults, preferred);
        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }

    function initPlayerDefaults(defaults) {
        Object.assign(playerDefaults, defaults);
    }

    function initPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(settings);
        const player = videoPlayer.addPlayer(video, playerOptions);
        videoWrapper.player = player;
    }

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        initPlayer(wrapper, config);
    }

    const defaults = {
        height: 207,
        width: 277
    };
    defaults.playerVars = {
        autoplay: 0,
        controls: 1,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
    };
    initPlayerDefaults(defaults);
    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);
}

One thing at a time please.

First can we deal with this JSLint situation:

After that we can then move on to playlists.

ok, sure.

Have you posted a link to the code that results in the above warnings?

It’s not there anymore, if I see it again I will let you know.

This is the code that is being worked on.

https://jsfiddle.net/vy2L4b7x/

Where the playlist is not visible.

I’ve found out how to recreate it, with the following code from https://jsfiddle.net/dqncvkpb/:

        const playerVars = Object.assign({},
            playerDefaults.playerVars,
            settings.playerVars
        );

The Object.assign parameters started out being given on the same line, so JSLint expects all of the function parameters to be given on the same line.

        const playerVars = Object.assign({}, playerDefaults.playerVars, settings.playerVars);

But that causes a different problem of being too long.

Reducing the size of the variable names is one way to deal with that, but then we lose clarity of what’s going on there.

Breaking down the line to multiple statements is another way to deal with things:

        const playerVars = Object.assign({}, playerDefaults.playerVars);
        Object.assign(playerVars, settings.playerVars);

But then it’s less clear what is happening, and why.

We could split it again to help make it more clear what is happening:

        const playerVars = {};
        Object.assign(playerVars, playerDefaults.playerVars);
        Object.assign(playerVars, settings.playerVars);

With that, it’s a lot easier to understand what is happening.
But if we’re going to do that then we can just split the original one across multiple lines too:

        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

And that is the final solution that I went with.

The error occurred due to the inconsistent formatting that you changed things to. Fortunately there are other valid ones to pick from.

1 Like

jslint said 0 errors when i put the code in like this.
https://jsfiddle.net/px8jtasg/

(function initCover() {
    function hide(el) {
        el.classList.add("hide");
    }

    function coverClickHandler(evt) {
        const cover = evt.currentTarget;
        hide(cover);
    }

    const covers = document.querySelectorAll(".jacket");
    covers.forEach(function addHandler(cover) {
        cover.addEventListener("click", coverClickHandler);
    });
}());

const videoPlayer = (function makeVideoPlayer() {
    const players = [];
    let playerVars = {};
    let player = null;

    const tag = document.createElement("script");
    tag.src = "https://www.youtube.com/player_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) {
        player = event.target;
        player.setVolume(100); // percent
        shufflePlaylist(player);
    }

    function onPlayerStateChange(event) {
        player = event.target;

        if (event.data === YT.PlayerState.PLAYING) {
            players.forEach(function pauseOtherVideos(player) {
                if (player !== event.target) {
                    player.pauseVideo();
                }
            });
        }

        if (playerVars.loop && event.data === YT.PlayerState.ENDED) {
            player.seekTo(playerVars.start);
        }
    }

    function addPlayer(video, settings) {
        const playerVarDefaults = {
            host: "https://www.youtube-nocookie.com",
            videoId: video.dataset.id
        };
        playerVarDefaults.events = {
            "onReady": onPlayerReady,
            "onStateChange": onPlayerStateChange
        };
        playerVars = Object.assign(playerVarDefaults, settings);
        players.push(new YT.Player(video, playerVars));
    }

    return {
        addPlayer
    };
}());

function loadPlayer(config) {
    const playerDefaults = {};

    function show(el) {
        el.classList.remove("hide");
    }

    function createPlayerOptions(settings) {
        function paramInOptions(opts, param) {
            if (settings[param] !== undefined) {
                opts[param] = settings[param];
                delete settings[param];
            }
            return opts;
        }

        const optionParams = ["width", "height", "videoid", "host"];
        const preferred = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, playerDefaults, preferred);
        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

        playerOptions.playerVars = playerVars;
        return playerOptions;
    }

    function initPlayerDefaults(defaults) {
        Object.assign(playerDefaults, defaults);
    }

    function initPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(settings);
        const player = videoPlayer.addPlayer(video, playerOptions);
        videoWrapper.player = player;
    }

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        initPlayer(wrapper, config);
    }

    const defaults = {
        height: 207,
        width: 277
    };
    defaults.playerVars = {
        autoplay: 0,
        controls: 1,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
    };
    initPlayerDefaults(defaults);
    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);
}

function onYouTubeIframeAPIReady() {
    loadPlayer({
        height: 207,
        playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
        target: ".jacket-left",
        width: 277
    });
    loadPlayer({
        height: 207,
        start: 4,
        target: ".jacket-middle",
        width: 277
    });
    loadPlayer({
        height: 207,
        target: ".jacket-right",
        width: 277
    });
}

Yes that’s right. The following results in zero problems.

        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

Moving the empty object up to the previous line creates those problems:

        const playerVars = Object.assign({},
            playerDefaults.playerVars,
            settings.playerVars
        );
1 Like

ok, so then it is fixed.

how does the playlist issue get fixed?
https://jsfiddle.net/px8jtasg/

Post 3 is where the playlist code worked last.

https://jsfiddle.net/xmpzbsqt/

Well lets follow it through.

    loadPlayer({
        height: 207,
        playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
        target: ".jacket-left",
        width: 277
    });

It’s received by loadPlayer as config:

function loadPlayer(config) {

and given to initPlayer in the coverClickHandler

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        initPlayer(wrapper, config);
    }

Because the loadPlayer code removes the cover property from config, it now only contains what should be the player options and playerVars, so the initPlayer function receives it as settings which combines both the options and playerVars:

    function initPlayer(videoWrapper, settings = {}) {

and those settings in there are given to createPlayerOptions:

        const playerOptions = createPlayerOptions(settings);

playlist still exists in settings, where the player options and playerVars are still mixed together.

The createPlayerOptions function receives the settings:

    function createPlayerOptions(settings) {

and it’s in here that the playerVars are assigned:

        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );
        playerOptions.playerVars = playerVars;

It’s in there that the problem occurs.

If we had used tests to develop the code, the problem would have been instantly reported to us.
But as things are without the tests, it’s hard to tell what the problem is.

I’ve given all of the information in the above code excerpts.
Can you tell what the problem is yet?

The problem comes from that the settings parameter doesn’t have playerVars as a property. The settings parameter instead smushes the options and playerVars together for convenience.

Because of that, there is no playerVars to get from settings.

        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings.playerVars
        );

Instead of settings.playerVars we should just use settings.

        const playerVars = Object.assign(
            {},
            playerDefaults.playerVars,
            settings
        );

We might even bring that back up to a single line:

        const playerVars = Object.assign({}, playerDefaults.playerVars, settings);

But that’s still considered to be too long. We can solve that by renaming playerDefaults to be just defaults. That means adjusting a few other things in the code, but we are left with the following:

        const playerVars = Object.assign({}, defaults.playerVars, settings);

And just to try and ensure that the same problem doesn’t happen again, a comment can be left about the changing nature of the settings parameter.

        // settings should now only consist of playerVars
        const playerVars = Object.assign({}, defaults.playerVars, settings);

The updated code is found at: https://jsfiddle.net/quabrytc/

1 Like

Should these be reversed?

function loadPlayer(config) {


    const defaults = {
        height: 207,
        width: 277
    };
    defaults.playerVars = {
        autoplay: 0,
        controls: 1,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
    };

    function show(el) {
        el.classList.remove("hide");
    }

This instead?

function loadPlayer(config) {

    function show(el) {
        el.classList.remove("hide");
    }

    const defaults = {
        height: 207,
        width: 277
    };
    defaults.playerVars = {
        autoplay: 0,
        controls: 1,
        disablekb: 1,
        enablejsapi: 1,
        fs: 0,
        iv_load_policy: 3,
        rel: 0
    };

It’s best to have defaults like that either at the top of the function or the bottom. Either of those places are easier to find than somewhere in the middle of it.