If I were to put 2 of these on a page, it would look like

JavaScript
82

What I mean are lines 94 and 103 of the code.

83

An easy first step is after indenting and spacing the code with jsBeautify or some other solution, to run the code through JSLint.

That will tell you about all the things that need to be fixed it.

1 Like
84 
  (function iife() {
      "use strict";

      function playButtonClickHandler() {
          var button = document.querySelector(".playButton2");
          var player = document.querySelector("#player2");
          document.querySelector(".playButton2 .initial").style.display = "none";
          player.volume = 1.0;
          if (player.paused) {
              button.classList.add("playing");
              document.querySelector(".playButton2 .play").style.display = "none";
              document.querySelector(".playButton2 .pause").style.display = "inline-block";
              player.play();
          } else {
              document.querySelector(".playButton2 .pause").style.display = "none";
              document.querySelector(".playButton2 .play").style.display = "inline-block";
              player.pause();
          }
          " onmouseover="
          var player = document.querySelector("#player2");
          player.isPlaying = function() {
              return player.paused === false;
          }
          if (player.isPlaying()) {
              document.querySelector(".playButton2 .speaker").style.display = "none";
              document.querySelector(".playButton2 .pause").style.display = "inline-block";
          }
          " onmouseout="
          var player = document.querySelector("#player2");
          player.isPlaying = function() {
              return player.paused === false;
          }
          if (player.isPlaying()) {
              document.querySelector(".playButton2 .pause").style.display = "none";
              document.querySelector(".playButton2 .speaker").style.display = "inline-block";
          }
      }
      var playButton = document.querySelector(".playButton2");
      playButton.addEventListener("click", playButtonClickHandler);
  }());
85

Bad assignment to ‘onmouseover’.
"onmouseover" =

87

Remove that line. It never did anything.

88

It was for the pause svg, hover.

Mouse goes over, pause shows.

mouse goes away, speaker shows.

89

Undeclared 'onmouseout'.
onmouseout =

90

Oh, so the event listener hasn’t even been set up for that code.

Damn, and my ride arrives soon too. Fortunately you have plenty of other code that assigns event listeners as an example of how to do it.

91

How come it works as is in the inline-javascript version then?

Shouldn’t they both work that same way?

Why doesn’t the inline-javascript version need extra code?

92

Let’s make a start at this.

Put the code in to functions, and assign those functions to the events:

    playButton.addEventListener("click", playButtonClickHandler);
    playButton.addEventListener("mouseover", playButtonMouseoverHandler);
    playButton.addEventListener("mouseout", playButtonMouseoutHandler);

Yay, and it’s all good.

93

And with JSLint cleaned up code, we have https://jsfiddle.net/j9anqqak/9/

94

The functions are still flawed though, because they keep on querying for the .playButton2 element when they already have it. But things like that can be dealt with when I return next week.

1 Like
95

So, I guess that, when you convert inline to regular javascript, you have to modify the extra code that goes beyond just play/pause.

96

Why do I say that they are flawed?

Imagine that the "playButton2" is renamed to something else, such as just "playButton". In how many places of the code will you have to go and make that change?

Will you even bother to do so? Not likely. So bad code remains bad because it’s too much of a minor effort to bother improving it.

Improving the code so that .playButton2 and other similar names occur only in one place of the scripting code, makes it much easier to respond to future developements.

2 Likes
97

I just put the code into a function:

function playButtonMouseoverHandler() {
    ...
}

And assigned that to the mouseover event:

playButton.addEventListener("mouseover", playButtonMouseoverHandler);

It’s not like it’s magic or anything.

1 Like
98

After all, when the code in an inline event handler, such as:

<div onclick="doSomething()">

The web browser ends up putting that inline code in to a function anyhow.

The onclick event of the div would have a function like this:

function onclick(event) {
  doSomething()
}

So we’re not really changing anything structurally by moving the inline event code out to a separate function. Even though it was inline with the HTML element, the web browser ends up moving it out to a separate function after all.

What we are achieving by moving the inline code down to be with the JavaScript code, is making the code easier to manage and work with instead.

99

You have to put numbers next to the class names otherwise the players on the page will look disfigured.

Unless you have a better solution.

100

Yes. A better solution is to use no numbers, as class names are designed to be used with many different elements that receive the same style.

101

Show me how 2 codes will work on the same page without numbers added to the class names.

102

Yes okay, I’ll take a look at it and see what can be done.