Questions about the YouTube player_api code


#162

I’m going to need to study, and read this thoroughly so I understand all of this.

Thank you.


#163

What’s the difference between

This way: Code 1
https://jsfiddle.net/hzyrfkwb/432/

And this way: Code 2
https://jsfiddle.net/hzyrfkwb/453/

Is one a better code than the other?


#164

Yes, everything that I did in the recent post is better.


#166

How come in this code these properties have to be specified up at the top?

What if I don’t want them added to the default settings?

start: 0,
end: 999999,
loop: true,

https://jsfiddle.net/hzyrfkwb/461/

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,

Would there be a way where I would only add them if they are needed?

Like how it is done here?

Doing this would remove a lot of unnecessary duplication.
https://jsfiddle.net/hzyrfkwb/432/

loadPlayer({
   target: ".jacketc",
   width: 600,
   height: 338,
   playerVars: {
      start: 200,
      end: 205,
      loop: true
   }
});

It doesn’t need to be stated in both the top:

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,

And also at the bottom:

loadPlayer({
   target: ".jacketc",
   width: 600,
   height: 338,
   playerVars: {
      start: 200,
      end: 205,
      loop: true
   }
});

These should only be added to loadPlayer if they are needed.
They shouldn’t be required to be at the top in the default settings.

        start: 0,
        end: 999999,
        loop: true,

These should only be the default settings:

         autoplay: 1,
         controls: 1,
         showinfo: 1,
         rel: 0,
         iv_load_policy: 3,
         cc_load_policy: 0,
         fs: 0,
         disablekb: 1
      };

These would only be added to loadPlayer if they are needed.
If they are not needed they don’t need to appear in the javascript.

        start: 0,
        end: 999999,
        loop: true,

How would I be able to implement this adjustment to the code?
https://jsfiddle.net/hzyrfkwb/453/

Where these playerVars aren’t needed in the default settings.
They only get added to loadPlayer if they are needed.

        start: 200,
        end: 205,
        loop: true,

That’s exactly how it works in the other code we did.
https://jsfiddle.net/hzyrfkwb/465/

How would I be able to implement that in the new, updated code?
https://jsfiddle.net/hzyrfkwb/453/


#167

Currently the settings object contains all the values mixed up with each other. width is with end, and start is with height. It’s a mess.

Those properties in the defaults area is a security measure. Only properties that appear in the defaults area will have their values updated.
That helps to prevent any values from the settings from ending up in the wrong place.


#169

Wait a second:

Instead of doing it this way:
https://jsfiddle.net/hzyrfkwb/482/

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,

loadPlayer({
  target: ".jacketc",
  width: 600,
  height: 338,
  start: 200,
  end: 205,
  loop: true
});
loadPlayer({
  target: ".playa",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playb",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playc",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playd",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playe",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playf",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playg",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playh",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playi",
  start: 8,
  end: 10,
  loop: false
});

It can be done this way instead, right?
https://jsfiddle.net/hzyrfkwb/483/

    function addVideo(video, settings) {
        const defaultSettings = {
            width: settings.width || 640,
            height: settings.height || 390,
            videoId: video.dataset.id,
            playerVars: {
                start: false,
                end: false,
                loop: false,


loadPlayer({
    target: ".jacketc",
    width: 600,
    height: 338,
    start: 200,
    end: 205,
    loop:true
});
loadPlayer({
    target: ".playa",
    start: 8,
    end: 10
});
loadPlayer({
    target: ".playb",
    start: 8,
    end: 40
});
loadPlayer({
    target: ".playc",
    start: 2,
    end: 90
});
loadPlayer({
    target: ".playd",
    start: 5,
    end: 60
});
loadPlayer({
    target: ".playe",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playf",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playg",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playh",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playi",
     start: 1,
     end: 50
});

#170

Assuming they both “work”, what benefits do you see with one over the other?


#171

With this code:
https://jsfiddle.net/hzyrfkwb/482/

loop has to be specified on all the loadPlayers:

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,
loadPlayer({
  target: ".jacketc",
  width: 600,
  height: 338,
  start: 200,
  end: 205,
  loop: true
});
loadPlayer({
  target: ".playa",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playb",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playc",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playd",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playe",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playf",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playg",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playh",
  start: 8,
  end: 10,
  loop: false
});
loadPlayer({
  target: ".playi",
  start: 8,
  end: 10,
  loop: false
});

And with this code:
loop is only being specified on the loadPlayers it’s being used on, instead of every single one.

I think this is good, but I want to see what @Paul_Wilkins has to say about it.
https://jsfiddle.net/hzyrfkwb/483/

    function addVideo(video, settings) {
        const defaultSettings = {
            width: settings.width || 640,
            height: settings.height || 390,
            videoId: video.dataset.id,
            playerVars: {
                start: false,
                end: false,
                loop: false,
loadPlayer({
    target: ".jacketc",
    width: 600,
    height: 338,
    start: 200,
    end: 205,
    loop:true
});
loadPlayer({
    target: ".playa",
    start: 8,
    end: 10
});
loadPlayer({
    target: ".playb",
    start: 8,
    end: 40
});
loadPlayer({
    target: ".playc",
    start: 2,
    end: 90
});
loadPlayer({
    target: ".playd",
    start: 5,
    end: 60
});
loadPlayer({
    target: ".playe",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playf",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playg",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playh",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playi",
     start: 1,
     end: 50
});

#172

That explains how they are different. The point of my question was why do you think one way or another is a better choice for what you are wanting to do with it.

You will find that in getting code to go from A to Z there will usually be more than one way to get there. Possibly as many different ways as there are programmers.

A suggestion. Rather than posting on multiple forums and asking for opinions on what you’ve been given on one at another,

Please. Take. Some. Time. To. Understand. What. The. Code. Is. Doing.

You need to “find your own place” in how you write code. If you keep on relying on others code and what others think of it you will have very slow going.


#173

To answer your question,

I don’t know if doing it this way is a good choice or not.

It was just something I thought of, instead of doing it the other way.

I just thought doing it this way would be better because then I wouldn’t need to specify
/ loop: false / in every single loadPlayer that loop isn’t being used in.

And this is what I came up with:
https://jsfiddle.net/hzyrfkwb/483/

    function addVideo(video, settings) {
        const defaultSettings = {
            width: settings.width || 640,
            height: settings.height || 390,
            videoId: video.dataset.id,
            playerVars: {
                start: false,
                end: false,
                loop: false,
loadPlayer({
    target: ".jacketc",
    width: 600,
    height: 338,
    start: 200,
    end: 205,
    loop:true
});
loadPlayer({
    target: ".playa",
    start: 8,
    end: 10
});
loadPlayer({
    target: ".playb",
    start: 8,
    end: 40
});
loadPlayer({
    target: ".playc",
    start: 2,
    end: 90
});
loadPlayer({
    target: ".playd",
    start: 5,
    end: 60
});
loadPlayer({
    target: ".playe",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playf",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playg",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playh",
    start: 1,
    end: 50
});
loadPlayer({
    target: ".playi",
     start: 1,
     end: 50
});

#174

Yes. that’s a good idea, but I’d want to make it clear in the default settings that loop is a custom addition, and not one that is defined by the youtube API.

   function addVideo(video, settings) {
        const defaultSettings = {
            ...
            playerVars: {
                start: false,
                end: false,
                loop: false, // custom setting, for onPlayerStateChange to loop video

People have wasted a lot of time trying to chase down official documentation about something that doesn’t exist. That comment protects you from the danger of falsely believing that loop is an official part of the youtube api.


#175

And it’s okay to have start, end to have false on them?

Which means that, any load players that have start, end specified in them, those would be enabled.

                start: false,
                end: false,

This would have worked too, though:

                start: 0,
                end: 0,

Would 0, or false be better there?

I think false is okay to be used there, right?
https://jsfiddle.net/hzyrfkwb/509/

        const defaultSettings = {
            width: settings.width || 640,
            height: settings.height || 390,
            videoId: video.dataset.id,
            playerVars: {
                start: false,
                end: false,
                loop: false, // custom setting, for onPlayerStateChange to loop video

#176

There is a difference:

False would be the the correct one, not 0.

start: false,
 end: false,

#177

Leave those as numbers. 0 for start, and something nice and long for end, such as 999999
999999 seconds lasts for 274 hours (more than 10 days), which is more than enough for anybody.


#180

This is what I was thinking:

If start, end are set to false in the default settings.

And end, start are not specified in loadPlayer.

End, start reverts back to its default settings which would be set to false.
Meaning, not enabled.

Same thing with loop.

If loop, is set to false in the default settings.

And loop is not specified in loadPlayer.

loop reverts back to its default settings which would be set to false.
Meaning, not enabled.


You don’t seem to think adding false to end, start is a good idea.

And this should be used instead:

end: 0,
start: 999999

But using false with loop is okay to use.

But just not with, end, start.

What’s the difference?

Couldn’t these be a custom addition also?
One that is not defined by the youtube API.
As you put it.

start: false,
end: false,

#181

Does the youtube api allow start or end to be false? Here’s what it says about the playerVars start parameter, for example:

source: https://developers.google.com/youtube/player_parameters#start
This parameter causes the player to begin playing the video at the given number of seconds from the start of the video. The parameter value is a positive integer.

That is why I recommend that you do not breach the requirements for that value and use an integer value. A value 0 is fully valid as the default start value. A value of false is not.


#182

Do you see anything in the code that might still need to be fixed, or adjusted?

I think it might need to be improved some, if it’s something you could take a look at.

or the code may look fine to you.


In another forum I asked how this would be done.

So loop, start, end don’t need to be specified at the top.
And it removes that duplication.

Does this do a good job of doing what it’s supposed to be doing,
so values don’t get mixed up in the wrong place?

What are your thoughts and opinion on how it was done?
https://jsfiddle.net/zt7anuL3/22/

    function combineSettings(oldSettings, newSettings) {
        const props = Object.keys(newSettings);
        let combinedSettings = props.reduce(function combine(combined, prop) {
            if (typeof(oldSettings[prop]) === "object") {
                const oldProp = oldSettings[prop] || {};
                const newProp = newSettings[prop] || {};
                combined[prop] = combineSettings(oldProp, newProp);
            } else {
                combined[prop] = newSettings[prop];
            }
            return combined;
        }, oldSettings);
        return combinedSettings;
    }

    function addVideo(video, settings) {
        const defaultSettings = {
            width: settings.width || 600,
            height: settings.height || 338,
            videoId: video.dataset.id,
            playerVars: {
                autoplay: 1,
                controls: 1,
                showinfo: 1,
                rel: 0,
                iv_load_policy: 3,
                cc_load_policy: 0,
                fs: 0,
                disablekb: 1
            },
            events: {
                "onReady": onPlayerReady,
                "onStateChange": onPlayerStateChange
            }
        };
        const updatedSettings = combineSettings(defaultSettings, settings);
        players.push(new YT.Player(video, updatedSettings));
    }

    function init(video, settings) {
        load.js("https://www.youtube.com/player_api").then(function() {
            YT.ready(function() {
                addVideo(video, settings);
            });
        });
    }
    return {
        init
    };
}());

function loadPlayer(opts) {
    "use strict";
    const show = (el) => el.classList.remove("hide");

    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");
        let settings = {};
        const {
            width,
            height,
            ...options
        } = opts;
        settings.width = width || 198;
        settings.height = height || 198;
        settings.playerVars = options.playerVars || options;
        videoPlayer.init(video, settings);
    }

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

For context on what was done to the code:

Done and done. I also took the time to do the object deconstruction thing I’d mentioned. Here’s how that code works:

(within loadPlayer() )

function initPlayer(wrapper) {
    const video = wrapper.querySelector(".video");
    // Create an EMPTY object, this will be our settings.
    let settings = {};
    // Strip out the width and height from the passed object, leave the rest
    const {width, height, ...options} = opts
    // default to 198x198 for multiple video players
    settings.width = width || 198;
    settings.height = height || 198;
    
    // the var 'options' contains everything BUT width and height.
    settings.playerVars = options.playerVars || options;
    
    videoPlayer.init(video, settings);
}

Doing the above means that width and height are not being added to both the settings object AND to playerVars. Now, within the combineSettings function, I changed it to this:

function combineSettings(oldSettings, newSettings) {
    // We want to add or update only those properties found on our 
    //  newSettings object -- this is our custom settings. So get ONLY
    //  those keys.
    const props = Object.keys(newSettings);
    let combinedSettings =  props.reduce(function combine(combined, prop) {
        // Is the current property a  nested object? If so, RECURSE!
        if (typeof(oldSettings[prop]) === "object") {
            const oldProp = oldSettings[prop] || {};
            const newProp = newSettings[prop] || {};
            combined[prop] = combineSettings(oldProp, newProp);
        } else {
            // Otherwise, simply add/update the property on  our defaults
            combined[prop] = newSettings[prop];
        }
        return combined;
    }, oldSettings);

    return combinedSettings;
}

Big change is that, as I am using the original oldSettings object, I really don’t care about its properties. I simply want to add/update those properties coming in from newSettings.

If you open the console, you will see the final settings object. Note that width and height are in the root level, while start and end are only in the playerVars IF THEY HAVE BEEN DEFINED.

Web Console:
https://jsfiddle.net/zt7anuL3/23/
console.log(JSON.parse(JSON.stringify(updatedSettings)));
image


#183

@Paul_Wilkins

How come jslint is telling me this is an error, when it’s not an error?
https://jsfiddle.net/zt7anuL3/18/

Is jslint out of date?
https://www.jslint.com/

const {
        width,
        height,
        ...options
    } = opts;

image

javascript spread operator:


#184

That’s because JSLint doesn’t just check for what’s valid. Instead JSLint only allows recommended techniques.

Destructuring on the left of an equals sign is not allowed by JSLint, because it’s too easily confused with statement blocks.

JSLint does allow though destructing of function parameters instead.

When something is not supported, such as destructured object assignment, do not make the mistake of thinking that it’s because it’s out of date. It’s not allowed because the case has not been successfully made that it results in good programming.


#185

How would this line be fixed then?
https://jsfiddle.net/zt7anuL3/18/

const {
        width,
        height,
        ...options
    } = opts;

Section of the code where the line is:

    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");
        let settings = {};
        const {
            width,
            height,
            ...options
        } = opts;
        settings.width = width || 198;
        settings.height = height || 198;
        settings.playerVars = options.playerVars || options;
        videoPlayer.init(video, settings);
    }

Context of what was done:

function initPlayer(wrapper) {
    const video = wrapper.querySelector(".video");
    // Create an EMPTY object, this will be our settings.
    let settings = {};
    // Strip out the width and height from the passed object, leave the rest
    const {width, height, ...options} = opts
    // default to 198x198 for multiple video players
    settings.width = width || 198;
    settings.height = height || 198;
    
    // the var 'options' contains everything BUT width and height.
    settings.playerVars = options.playerVars || options;
    
    videoPlayer.init(video, settings);
}