That handler is all good for now. But it’s best if functions have only a single responsibility. Each function should have one and only one reason for change. The handler for example, only had to worry about passing on the button from the event.
It’s like in business. When you have a person that has multiple masters, that person ends up in trouble because he cannot keep both of them happy at the same time.
Here’s some good info about the Single Responsibility Principle and Separation of Concerns .
asasass
November 22, 2017, 6:01am
22
It originally looked like this:
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
function getPlayButton(el) {
while (el.classList.contains("playButton") === false) {
el = el.parentNode;
}
return el;
}
function playButtonClickHandler(evt) {
var button = getPlayButton(evt.target);
togglePlayButton(button);
}
function upTo(el, className) {
while (el.classList.contains(className) === false) {
el = el.parentNode;
}
return el;
}
var button = upTo(evt.target, "playButton");
}());
I removed this piece of code from it:
function upTo(el, className) {
while (el.classList.contains(className) === false) {
el = el.parentNode;
}
return el;
}
var button = upTo(evt.target, "playButton");
asasass
November 22, 2017, 6:10am
23
Can these 2 be combined? That’s what I was just thinking.
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
function playButtonClickHandler(evt) {
var button = getPlayButton(evt.target);
togglePlayButton(button);
}
asasass
November 22, 2017, 6:14am
24
I want to fix this one so it does that.
But it’s best if functions have only a single responsibility.
That wouldn’t be a good idea.
What makes you think that it would be a good idea to combine the two?
asasass
November 22, 2017, 6:22am
26
I have no idea.
But what would work, to do what you said?
asasass
November 22, 2017, 6:28am
28
It should be able to work like this.
function getPlayButton(el) {
while (el.classList.contains("playButton") === false) {
el = el.parentNode;
}
return el;
}
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
That would work with a single player. It takes only a simple change for it to work with any number of players.
asasass
November 22, 2017, 6:31am
30
What would the change be?
asasass
November 22, 2017, 6:32am
31
Multiplayer uses this:
var playButtons = document.querySelectorAll(".playButton");
playButtons.forEach(function addPlayButtonHandler(el) {
el.addEventListener("click", playButton);
Single player would use: ???
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
The following code using querySelector only ever gets the first play button
var playButton = document.querySelector(".playButton");
The following code using querySelectorAll gets all of the play buttons
var playButtons = document.querySelectorAll(".playButton");
When you have all of the play buttons, you can then loop through them using the forEach method.
playButtons.forEach(function (playButton) {
playButton.addEventListener("click", playButtonClickHandler);
});
But beware, for removing the playButton variable will have an impact on some of your code, as we discussed earlier and fixed too. When code wrongly uses the playButton as a side-effect, that is a separate issue that is easy to fix.
asasass
November 22, 2017, 6:36am
33
What about for this?
Single player would use: ???
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
asasass
November 22, 2017, 6:36am
34
What else gets added?
To this?
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
function getPlayButton(el) {
while (el.classList.contains("playButton") === false) {
el = el.parentNode;
}
return el;
}
var playButton = document.querySelector(".playButton");
playButton.addEventListener("click", playButtonClickHandler);
}());
Yes, that would be correctly used for a single player.
asasass
November 22, 2017, 6:38am
36
It’s not working on a single player though.
Did I do something wrong in the way I set it up?
Go back to the browser console. What does it tell you?
asasass
November 22, 2017, 6:43am
38
ReferenceError: playButtonClickHandler is not defined
It looks like you accidentally deleted that function, when it is needed.
