Reputation: 102
I have two different video players in a page and right now they are working as intended, however, the code itself looks silly and unneccesarily repetitive, but I am not sure how to make it work while removing the basically copied block of code.
There are two divs with individual videos:
First
<video class='video2' poster='./assets/video2.jpg' src='./assets/video2.mp4'></video>
<div class='buttons'>
<button id='playPause2'><img src='./assets/play.png'></button>
</div>
Second
<video class='video1' poster='./assets/videoPoster.jpg' src='./assets/video1.mp4'></video>
<div class='buttons'>
<button id='playPause1'><img src='./assets/play.png'></button>
</div>
And the JS that runs the videos is as follows
var video1 = document.querySelector('.video1');
var video2 = document.querySelector('.video2');
var btn1 = document.getElementById('playPause1');
var btn2 = document.getElementById('playPause2');
function togglePlayPause1() {
if (video1.paused) {
btn1.className = 'pause';
video1.play();
} else {
btn1.className = 'play';
video1.pause();
}
}
function togglePlayPause2() {
if (video2.paused) {
btn2.className = 'pause';
video2.play();
} else {
btn2.className = 'play';
video2.pause();
}
}
btn1.onclick = function() {
togglePlayPause1();
};
btn2.onclick = function() {
togglePlayPause2();
};
As I said it works, but looks very silly, basically the same code repeating twice. How can I make this cleaner, while still holding the same functionality?
Upvotes: 0
Views: 46
Reputation: 759
I have not tested it, but I think it should work. Can you give it a try and let me know?
[1, 2]
.map(function(n) { return document.getElementById('playPause' + n); })
.forEach(function(btn, i) {
btn.onclick = function() {
var video = document.querySelector('.video' + i);
btn.className = video.paused ? 'play' : 'pause';
if (video.paused)
video.play();
else
video.pause();
};
});
However, I would suggest an alternative solution at the cost of brevity of solution.
I would suggest going for a proper naming convention when naming your buttons and videos. For instance, the IDs can be v1_video
and v1_btn
. Yes, I also recommend using IDs instead of class names for identifying different videos.
Following such a convention will make your code more readable and extensible for more complex use-cases like fetching the list of videos from an API.
['v1', 'v2'] // This array can instead be a list of IDs being fetched from an API
.map(function(label) {
return {
Video: '#' + label + '_video',
Button: '#' + label + '_btn'
};
})
.forEach(function(pair, i) {
var btn = document.querySelector(pair.Button);
btn.onclick = function() {
var video = document.querySelector(pair.Video);
btn.className = video.paused ? 'play' : 'pause';
if (video.paused)
video.play();
else
video.pause();
};
});
Upvotes: 0
Reputation: 351403
You could pass the CSS selectors for both types of elements to a function. With some ternary operator, you can also reduce a bit the code:
function manageVideo(videoSelector, btnSelector) {
let video = document.querySelector(videoSelector);
document.querySelector(btnSelector).onclick = function() {
this.className = video.paused ? "pause" : "play";
video[video.paused ? "play" : "pause"]();
};
}
manageVideo(".video1", "#playPause1");
manageVideo(".video2", "#playPause2");
Upvotes: 0
Reputation: 31
var video1 = document.querySelector('.video1');
var video2 = document.querySelector('.video2');
var btn1 = document.getElementById('playPause1');
var btn2 = document.getElementById('playPause2');
function togglePlayPause(video) {
if (video.paused) {
this.className = 'pause';
video.play();
} else {
this.className = 'play';
video.pause();
}
}
var videos = [video1,video2];
[btn1,btn2].forEach(function(el,i){
el.onclick = function()
{
togglePlayPause(videos[i]);
}
})
Upvotes: 2