JavaScript视频播放器

Question

我为一个视频播放器制作了一些JavaScript代码，但我在编码方面的经验是使用C#，语言之间的差异和我缺乏经验使我怀疑我没有遵循正确的指导方针。

我只是想知道，如果有人做了一次，他们会有好的批评和指点，他们可以提供。我已经注释了密码来解释我的想法。

let activateDisc = false;
let playingDisc = false;
let animatingDisc = false;

let volumeOn = true;

//Get the elements on the page.
let page = document.querySelector("#page");
let cover = document.querySelector("#videocover");
let disc = document.querySelector("#disc");

let playIcon = document.querySelector("#playicon");
let pauseIcon = document.querySelector("#pauseicon");

let volumeOnIcon = document.querySelector("#volumeon");
let volumeOffIcon = document.querySelector("#volumeoff");

let videoContainer = document.querySelector("#videocontainer");
let video = document.querySelector("video");
let volumeButton = document.querySelector("#volume");
let videoSlider = document.querySelector("#videoslider");

//Set it up so clicking the disc will start the video and the video ending will eject the disc.
disc.onclick = function() {insertDisc();}
video.onended = function() {ejectDisc();}

//Attach each element to their equivalent function.
document.querySelector("#restart").onclick = function() {resetVideo();}
document.querySelector("#rewind").onclick = function() {rewindVideo();}
document.querySelector("#skip").onclick = function() {skipVideo();}
document.querySelector("#end").onclick = function() {ejectDisc();}

//For play if the disc isn't in, we want to insert it.
document.querySelector("#play").onclick = function() {
    if (!activateDisc) {
        window.scrollTo(0, 0);
        insertDisc();
    }
    else {
        //Then we want to check either pause or play the video.
        if (!playingDisc){
            playVideo();
        }
        else {
            pauseVideo();
        }
    }
}

//The volume button checks if the volume is on and calls the correct method.
volumeButton.onclick = function() {
    if (volumeOn){
        turnVolumeOff();
    }
    else {
        turnVolumeOn();
    }
}

//Every time the current playing position of the video updates we'll set the slider accordingly.
video.ontimeupdate = function() {
    //The slide is set as the current time in the video over the total length of the video multiplied by the max slider value.
    videoSlider.value = video.currentTime / video.duration * videoSlider.max;
}

//When the mouse is over the video we want to show the video slider and volume button.
videoContainer.addEventListener('mouseover', function() {
    videoSlider.style.opacity = 1;
    volumeButton.style.opacity = 1;
},false);

//When the mouse leaves the video we want to hide the video slider and volume button.
videoContainer.addEventListener('mouseout', function() {
    videoSlider.style.opacity = 0;
    volumeButton.style.opacity = 0;
},false);

//When the slider value is changed we want to set the video time to the slider value.
videoSlider.addEventListener("input", function() {
    //The current time in the video is set as the slider value over the max slider value multiplied by the total length of the video.
    video.currentTime = videoSlider.value / videoSlider.max * video.duration;
}, false);

//Inserting the disc.
function insertDisc() {
    //If the disc is already in eject it.
    if (!activateDisc){
        //If the disc is already animating we return and do nothing.
        if (animatingDisc) return;
        animatingDisc = true;
        activateDisc = true;
        disc.style.animation = "playdisc 1s linear";
        //Wait for the disc to be fully inserted and play the video.
        setTimeout(() => {
            playDiscAnimation();
            scrollToWithOffset(disc, -80);
            hideElement(cover);
            video.style.animation = "flicker 500ms infinite alternate-reverse";
            playVideo();
        }, 1000);
    }
    else {
        ejectDisc();
    }
}

//Ejecting the disc.
function ejectDisc() {
    //If the disc is in we can eject it.
    if (activateDisc){
        if (animatingDisc) return;
        animatingDisc = true;
        //Reset the video back to its original state and eject it.
        pauseVideo();
        resetVideo();
        turnVolumeOn();
        video.style.animation = "";
        showElement(cover);
        ejectDiscAnimation();
        window.scrollTo(0, 0);
        //Replay the disc bouncing animation.
        setTimeout(() => {
            disc.style.animation = "bounce 300ms infinite alternate-reverse";
            activateDisc = false;
        }, 1000);
    }
}

//Play the video.
function playVideo() {
    if (!activateDisc) return;
    playTvAnimations();
    playingDisc = true;
    video.play();
}

//Pause the video.
function pauseVideo() {
    if (!activateDisc) return;
    pauseTvAnimations();
    playingDisc = false;
    video.pause();
}

//Reset the video back to zero.
function resetVideo() {
    if (!activateDisc) return;
    video.currentTime = 0;
}

//Rewind the video 5 milliseconds.
function rewindVideo() {
    if (!activateDisc) return;
    video.currentTime -= 5;
}

//Skip 5 milliseconds in the video.
function skipVideo() {
    if (!activateDisc) return;
    video.currentTime += 5;
}

//Turn the volume on.
function turnVolumeOn() {
    video.volume = 1;
    hideElement(volumeOffIcon);
    showElement(volumeOnIcon);
    volumeOn = true;
}

//Turn the volume off.
function turnVolumeOff() {
    video.volume = 0;
    hideElement(volumeOnIcon);
    showElement(volumeOffIcon);
    volumeOn = false;
}

//Play the disc insertion and playing animations.
function playDiscAnimation() {
    disc.style.animation = "spindisc 300ms infinite linear";
    page.style.backgroundColor = "#333030";
    animatingDisc = false;
}

//Play the disc ejection animation.
function ejectDiscAnimation() {
    disc.style.animation = "playdisc 1s reverse";
    disc.style.animationPlayState = "running";
    page.style.backgroundColor = "#b8c9dd";
    animatingDisc = false;
}

//Play the tv animation.
function playTvAnimations() {
    disc.style.animationPlayState = "running";
    hideElement(playIcon);
    showElement(pauseIcon);
}

//Pause the tv animations.
function pauseTvAnimations() {
    disc.style.animationPlayState = "paused";
    hideElement(pauseIcon);
    showElement(playIcon);
}

//Show an element by setting its display to block.
function showElement(element) {
    element.style.display = "block";
}

//Hide an element by setting its display to none.
function hideElement(element) {
    element.style.display = "none";
}

//Scroll towards an element with a Y offset.
function scrollToWithOffset(element, yOffset = 0){
    var scrollY = element.getBoundingClientRect().top + window.pageYOffset + yOffset;
    window.scrollTo(0, scrollY);
}

Answer 1

首先，恭喜可读的代码。了解正在发生的事情并不难，似乎对其他人来说也很容易理解。

我有几个建议给你。大多数情况下，这只会使您的JavaScript更加惯用，但也有一些质量改进的可能性。

让

对Const对Var

在现代JavaScript中，您可能不想使用var。它使变量声明受制于吊装，这可以通过使变量名在您不希望出现的地方可用来创建不可预测的行为，特别是当来自另一种语言时。

我的建议是始终使用const，除非您知道要重新分配变量，然后使用let。这意味着，在const情况下，块中的变量名总是引用相同的对象/值，或者在let的情况下，您可以重新分配它。这两种选择都是对下一个阅读代码的人的一个信号，可以帮助提高代码的可维护性。此外，如果您正在整理您的项目，您的代码质量工具可以警告您，如果您试图无意地重新分配一个const。更多的这里。

let的大部分用途应该是const。这似乎是你唯一使用var的地方。

单行If-然后

我建议你不要这样做：

if (!activateDisc) return;

人们认为它既短又漂亮，但在这种情况下，快捷键可能会在以后造成问题。为if-then语句的正文使用块范围以避免自动分号插入的危害，如下所示：

if (!activateDisc) {
    return;
}

将事件处理程序命名为

在浏览器中，如果不再需要事件处理程序，您希望能够删除它们。如果你不这样做，事情就会发生，这是你预料不到的。由于这个原因，并且由于它根本不是必要的，所以不应该将事件处理程序函数包装为匿名函数。您可能应该使用addEventListener而不是分配给onclick，因此如果需要的话，可以为单击事件分配多个事件处理程序。使用命名函数而不是匿名函数将为您提供一个引用，以便您以后可以删除处理程序。

这个例子：

document.querySelector("#restart").onclick = function() {resetVideo();}

应：

document.querySelector("#restart").addEventListener("click", resetVideo);

这样，如果您需要这样做，以后可以这样做：

document.querySelector("#restart").removeEventListener("click", resetVideo);