Yep. The one thing that was done in that whole process was to replace a combination of addEventListener +removeEventListener with only a single addEventListener.
Everything else was responding to issues from that restructure, using best-practice techniques.
Itâs easier to make those adjustments when code is well-structured. Poorly structured code becomes a nightmare to make those adjustments. Fortunately here the code is mostly well structured, so it wasnât too tough to make those adjustments.
A fundamental principle when restructuring code is to test early, test often. When small steps are taken each time before testing, it tends to only be a small and easy update to get things working again.
Thereâs only one further thing that I would do in terms of the restructure with this event handler, and that is to split up the fadeResetHandler. Right now itâs checking if something should be done, then doing that thing. The handler should only do the checking before calling a separate function. That means moving the rest of the code after the if statement into a separate function.
The fadeResetHandler is actually an animationEndHandler and should be renamed to that instead.
That allows us to move the rest of the code after the if statement into a separate function called fadeReset.
That way instead of having fadeReset being called after the if statement:
if (animationName !== "fadingOut") {
return;
}
fadeReset();
We can invert the condition in the if statement and have fadeReset being called from inside of it.
if (animationName === "fadingOut") {
fadeReset();
}
Why that is beneficial is that it makes it easier to extend the code later on, should other animations other than fadingOut require other code being added there too.
Yes thatâs right. It is the proper job of an event handler to be like a policeman at an intersection, to direct traffic where it needs to go. That way complexity is dramatically reduced.
Iâm wondering if Paul Wilkins will be included in the copyright for this.
It is a great tutorial but I always found that I was more likely to retain information when I solved a problem myself rather than having the solution served up on a platter.
Somehow I doubt it, but it would be an appropriate attribution if it occurred.
Thatâs why I now try to explain things in a descriptive detail, so that appropriate terminology and concepts can be engaged with. Because I am not on the clock, I have as much time as is necessary to try and help those concepts to be learned.
You should provide your own custom badge for inclusion on the associated website as a token of gratitude for the time you have taken to help people like this.
That originalEvent variable really is a problem, so letâs see what we can do about that.
In the coverClickHandlerContinue function the originalEvent variable is renamed to be cover. Itâs not a cover though because the showCovers function calls it a playButton, and from that playButton the showCovers function gets the cover.
So the first change that needs to be done is that the coverClickHandlerContinue function parameter should be renamed to be playButton, with code in that function being renamed to use playButton instead.
The showCovers function doesnât show all of the covers either. Instead it just shows one cover, so the showCovers function needs to be renamed to showCover instead. The originalEvent variable can also be more correctly renamed to be currentPlayButton instead.
The removal of initial-fade shouldnât be in the coverClickHandlerContinue function either. Instead itâs more appropriate for that to be moved into the if statement that checks for initial-fade.
The coverClickHandlerContinue is now more easily understood to do things that are needed when showing the cover. We can move all of the coverClickHandlerContinue code (all except for the showCover line of course) into the showCover function. Any references to coverClickHandlerContinue can now be renamed to showCover, allowing us to delete the coverClickHandlerContinue function.
Good one. That animationEndHandler2 shouldnât have a number on it either. There doesnât seem to be an animationEndHandler that already exists, so rename animationEndHandler2 to be animationEndHandler
Will this global variable be removed, or is that staying? let currentPlayButton = {};
One wasnât needed with the manageUI function, not sure why it would be needed in one and not the other. Maybe they both canât be set up the same way.
function animationEndHandler(evt) {
const animationName = evt.animationName;
if (animationName === "initial-fade") {
body.classList.remove("initial-fade");
showCover(currentPlayButton);
}
}
function init(selectors) {
body.addEventListener("animationend", animationEndHandler);
}
How animationend was added to each.
animationEndHandleradded tomanageCover
Maybe this one needs let currentPlayButton = {};
const manageCover = (function makeManageCover() {
const body = document.body;
let currentPlayButton = {};
const animationName = evt.animationName;
if (animationName === "initial-fade") {
body.classList.remove("initial-fade");
showCover(currentPlayButton);
}
}
function coverClickHandler(evt) {
currentPlayButton = evt.currentTarget;
body.classList.add("initial-fade");
}
function init(selectors) {
body.addEventListener("animationend", animationEndHandler);
}
animationEndHandleradded tomanageUI
const manageUI = (function makeManageUI() {
const body = document.body;
function animationEndHandler(evt) {
const animationName = evt.animationName;
console.log(animationName);
if (animationName === "fadingOut") {
fadeReset();
}
}
function fadeReset() {
body.classList.remove("fadingOut");
resetBackground("body");
resetCurtains(".with-curtain");
showAllButtons(".hide");
resetButtons(".outer");
}
function resetPage() {
body.classList.add("fadingOut");
}
function exitClickHandler() {
resetPage();
}
function init() {
body.addEventListener("animationend", animationEndHandler);
}
The currentPlayButton variable is a temporary storage place, so that while waiting for the animation to end, it can be remembered which play button was activated. Thatâs the only reason for the use of currentPlayButton
Another possible solution is to add a dataset value onto the body, allowing us to remove the currentPlayButton variable. That alternative solution could be explored too if youâre up for it.
I thought that it would be easy, but on further investigation dataset only lets you store a string value. Itâs not possible to store a reference to a page element in a dataset, storing classname information is a terrible idea because parts of that change frequently.
Using a unique identifier is a possible solution but that means adding even more to the HTML code and other scripting code to get the appropriate unique id. Using a dataset is a bad motivation to start using unique ids when theyâre not being already used, because we already have a good working solution, so datasets donât look to be a viable solution.
Thatâs okay - for figuring out what doesnât work is an important part of the process as well.