0
\$\begingroup\$

The following code loads a YouTube video inside an iframe, where the tricky part is to sync the onYouTubeIframeAPIReady and on('shown.bs.modal') functions. The code works well in Firefox and decently in Chrome, but in Safari it is quite buggy. Any hints towards improvement would be appreciated.

jQuery( document ).ready(function() {
var tag = document.createElement('script');
tag.src = "https://www.youtube.com/iframe_api";
var firstScriptTag = document.getElementsByTagName('script')[0];
firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);

jQuery("#video_modal").on('shown.bs.modal', function() {
    if(typeof player.playVideo == 'function') {
        player.playVideo();
    } else {
        var fn = function(){
            player.playVideo();
        };
        setTimeout(fn, 200);
    }
});
jQuery("#video_modal").on('hidden.bs.modal', function() {
    player.stopVideo();
});
});

var player;

function onYouTubeIframeAPIReady() {
player = new YT.Player('player', {
    videoId: 'zg8KE6bEP50',
    origin: 'http://rollnstroll.se',
    width: '640',
    height: '390'
});
}
\$\endgroup\$
2
\$\begingroup\$

Don't remove the indentation inside of code blocks so that the code is on the same line as the signature. It makes the code harder to read as it's hard to piece together which } goes to which signature.

For example, this:

function onYouTubeIframeAPIReady() {
player = new YT.Player('player', {
    videoId: 'zg8KE6bEP50',
    origin: 'http://rollnstroll.se',
    width: '640',
    height: '390'
});
}

Would become this:

function onYouTubeIframeAPIReady() {
    player = new YT.Player('player', {
        videoId: 'zg8KE6bEP50',
        origin: 'http://rollnstroll.se',
        width: '640',
        height: '390'
    });
}

var tag = document.createElement('script');

The name "tag" is extremely general and gives little aid in defining the use of the variable. Since in the next line you give it a script source, I recommend calling it something that defines the script source.

Here is what I came up with

var ytIframeAPI = document.createElement("script");

Why are you inserting a script into the HTML via JavaScript? Why don't you just put the script tag in your HTML so it is loaded to start?

That way, you don't have to wait for the script to load from the source after the page has loaded; the script will load as the page is loading.


var fn = function(){
    player.playVideo();
};
setTimeout(fn, 200);

Again, like "tag", "fn" means nothing to the variable; I know that it is a function. The variable name does not have to tell me.

You have two options here:

  1. Give it a better name.

  2. Use an anonymous function.

Since the only purpose of that function is to use it in setTimeout, you can just use an anonymous function in place of fn.

Here is what I mean:

} else {
    setTimeout(function() {
        player.playVideo();
    }, 200);
}

See? I didn't even have to give the function a name.


This might just be a typo, but this confuses me:

if(typeof player.playVideo == 'function') {
    player.playVideo();
} else {
    var fn = function(){
        player.playVideo();
    };
    setTimeout(fn, 200);
}

If player.playVideo wasn't a function, it's going to go to else. Then, you call player.playVideo. Well, if it wasn't a function, how are you supposed to call it?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.