You still are not answering my question.
Do you need to find the video container every time a button is clicked? Please explain your answer of why you would need or not need to do so.
Yes I would because it is all housed under 1 container now.
<div class="container play1 with-curtain">
<div class="inner-container curtain ">
<div class="ratio-keeper">
<div class="wrap">
<div class="video video-frame"></div>
</div>
</div>
<button class="exit" type="button" title="Exit" aria-label="Close"></button>
</div>
</div>
The correct answer is No. See if you can figure out why.
It’s only looking for 1 element, not multiple?
This is the one that uses the least amount of code.
document.querySelectorAll('button.cover').forEach(function(button) {
button.addEventListener('click', function (button) {
document.querySelector('.video').dataset.id = button.target.dataset.id;
});
});
This one also:
const playButtons = document.querySelectorAll('button.cover');
function buttonClick(button) {
document.querySelector('.video').dataset.id = button.target.dataset.id;
}
for (let i = 0; i < playButtons.length; i++) {
const button = playButtons[i];
button.addEventListener('click', buttonClick);
}
That is part of the reason, but not the main reason why.
Let me ask the question a different way. Which of the following methods seems more efficient with respect to calling the same person on the phone everyday over a week.
Monday - Look up the person’s phone number on Google. Call the person.
Tuesday - Look up the person’s phone number on Google. Call the person.
Wednesday - Look up the person’s phone number on Google. Call the person.
Thursday- Look up the person’s phone number on Google. Call the person.
Friday - Look up the person’s phone number on Google. Call the person.
Saturday - Look up the person’s phone number on Google. Call the person.
Sunday - Look up the person’s phone number on Google. Call the person.
OR
Monday - Look up the person’s phone number on Google. Save number under phone contacts app. Call the person using the phone contacts app.
Tuesday - Call the person using the phone contacts app.
Wednesday - Call the person using the phone contacts app.
Thursday- Call the person using the phone contacts app.
Friday - Call the person using the phone contacts app.
Saturday - Call the person using the phone contacts app.
Sunday - Call the person using the phone contacts app.
Monday - Look up the person’s phone number on Google. Save number under phone contacts app. Call the person using the phone contacts app.
I agree. So, how could you apply this type of logic to your all of your posted code segments above? Just rewrite one of them for now applying this logic.
Something like this would be written differently?
I don’t understand how so.
document.querySelectorAll('button.cover').forEach(function(button) {
button.addEventListener('click', function (button) {
document.querySelector('.video').dataset.id = button.target.dataset.id;
});
});
Rewrite the code above without forcing JavaScript to search and find the element with class="video" every time a button is clicked. Currently, you are telling JavaScript to go find it every time a button is clicked. This is the equivalent of Googling for the person’s phone number everyday. It is a waste of time and system resources.
1 Like
ok, thank you for that, I will see if I am able to do this.
I really don’t understand why you so vehemently oppose the idea of learning the basics of HMTL, CSS, and JavaScript?
You have been working on the same project for 4 years with little progress. If you had spent 6-12 months on the basics, you could be working on much more complex projects by now.
If I understand you correctly, you are wanting me to rewrite that code without the .video element included in it?
Do I have that correct?
Yes. That is the goal.
1 Like
Do you just mean to cache the element in a variable outside of the event handler function?
Can you elaborate a little bit more please?
Broadly speaking, caching selectors is not a bad idea. However, there are instances when you might need to re-query the DOM for an element. Just as long as you understand what and when you are caching the element it is fine.
In the latest code you posted, the parameter to the click handler function should not be button it is an event object. Calling it a button is misleading, especially when used inside a scope that already has a button identifier. You are shadowing the outer button variable (forEach callback parameter) by creating a new variable with the same name inside an inner scope.
In this small piece of code, it might not be much of an issue as we can see that the second button identifier is in fact an event object and we can also tell by how it is being accessed (.target).
However, shadowing should be avoided, and naming things what they really are is important.
1 Like
document.querySelectorAll('button.cover').forEach(function(button) {
button.addEventListener('click', function (event) {
document.querySelector('.video').dataset.id = event.target.dataset.id;
});
});
Like this?
const myVideo = document.querySelector('.video');
document.querySelectorAll('button.cover').forEach(function(button) {
button.addEventListener('click', function(event) {
myVideo.dataset.id = event.target.dataset.id;
});
});
It might seem like a small detail but it all adds up. Your code is now more readable and less confusing. I can scan it much faster and not get hung up on it for even a split second.
1 Like
I am glad you figured out a more readable/efficient solution.
Now, I have another question for you. In one of the fiddles, you had 100 buttons where when you clicked on a button, a different video would show. From a UX standpoint, why would you want to display 100 buttons for a user? There was no indication of what video was to be played before clicking on the button, so why show that many at once?
Why so many buttons, I was seeing what was possible.
I have some ideas.
It looks much cleaner now: https://jsfiddle.net/m5bz9nLu/
Now only 1 container instead of 100.
I lowered it down to 96
This part looks much better.
No longer 100 lines inside here:
function onYouTubeIframeAPIReady() {
//For single videos.
for (let i = 1; i <= 96; i++) {
players.add(".playSingle" + i);
}
//Custom added playervars
Playlists, repeats, set different properties.
/*players.add(".customPlayer", {
playerVars: {
playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
}
});*/
}