Two versions of a playButton Binding Audio With a grid link structure


#66

Basing it off of your example, I did something like this but now I'm confused.

I'm going to need to wait till you get back to set this up the proper way.

<script>
  (function iife() {
    "use strict";

    function playButtonClickHandler() {
        var button = document.querySelector(".playButton");
        var player = document.querySelector("#player");
        player.volume = 1.0;
        if (player.paused) {
            button.classList.add("playing");
            document.querySelector(".playButton .play").classList.add("hide");
            document.querySelector(".playButton .pause").classList.remove("hide");
            player.play();
        } else {
            button.classList.remove("playing");
            document.querySelector(".playButton .play").style.display = "inline-block";
            document.querySelector(".playButton .pause").style.display = "none";
            player.pause();
        }
    }



    function playButton(button, player, play, pause) {
        ...
            play.classList.add("hide");
            pause.classList.remove("hide");
        ...
            play.classList.remove("hide");
            pause.classList.add("hide");
        ...
    }


    function playButtonClickHandler() {
        var button = evt.target;
        playButton({
            button: button,
            player: document.querySelector("#player"),
            play: button.querySelector(".play"),
            pause: button.querySelector(".pause")
        });
    }

#69

I haven't looked at any of the code that you've posted here because I am not in to interpreting chicken gizzards for signs.

Please explain in words what you want to achieve, and give a link to code that is currently not doing what you want to achieve.


#70

I have 2 versions of this code.

1st Version
One where when you click on it, it starts playing right away.

2nd Version
And another where when you click on it, it doesn't start playing right away.


1st Version
Is there anything wrong with how the javascript is set up?
https://jsfiddle.net/7ourwfrg/

<script>
  (function iife() {
    "use strict";
    document.querySelector(".myLink1").classList.add("hide");

    function playButtonClickHandler() {
      document.querySelector(".myLink1").classList.remove("hide");
      var button = document.querySelector(".playButton1");
      var player = document.querySelector("#player1");
      document.querySelector(".playButton1 .initial1").style.display = "none";
      player.volume = 1.0;
      if (player.paused) {
        button.classList.add("playing");
        document.querySelector(".playButton1 .play1").style.display = "none";
        document.querySelector(".playButton1 .pause1").style.display = "inline-block";
        player.play();
      } else {
        document.querySelector(".playButton1 .play1").style.display = "inline-block";
        document.querySelector(".playButton1 .pause1").style.display = "none";
        player.pause();
      }
    }
    var playButton = document.querySelector(".playButton1");
    playButton.addEventListener("click", playButtonClickHandler);
  }());

</script>

2nd Version:
Is there anything wrong with how the javascript is set up? And if there is, what's wrong with it?
https://jsfiddle.net/02me2fvk/6/

<script>
  (function iife() {
    "use strict";

    document.querySelector(".myLink1").classList.add("hide");
    var playButton = document.querySelector(".playButton1");
    playButton.addEventListener("click", firstClick);
    var player = document.querySelector("#player1");

    function firstClick() {
      document.querySelector(".myLink1").classList.remove("hide");
      var button = document.querySelector(".playButton1");
      document.querySelector('.playButton1 .initial1').style.display = 'none';
      playButton.addEventListener("click", playButtonClickHandler);
      document.querySelector(".playButton1 .play1").style.display = "inline-block";
      document.querySelector(".playButton1 .pause1").style.display = "none";
      button.classList.add("playing");
    }

    function playButtonClickHandler() {

      player.volume = 1.0;
      if (player.paused) {
        document.querySelector(".playButton1 .play1").style.display = "none";
        document.querySelector(".playButton1 .pause1").style.display = "inline-block";
        player.play();
      } else {
        document.querySelector(".playButton1 .play1").style.display = "inline-block";
        document.querySelector(".playButton1 .pause1").style.display = "none";
        player.pause();
      }
    }


  }());

</script>

#71

Please take your CSS code over to the CSS forum.

When you have any questions relating to JavaScript, that then is a good time to post in the JavaScript channel about your code.


#72

I edited the post.


#73

The click handler should have one one or two lines of code in it, handing off control to well-named functions. That helps to give you a better idea of what the code is supposed to do.

Too many class names

  • myLink1 is used twice when assigning it to a variable results in less complexity.
  • playButton1 is used seven times!! Only once is all that's needed.
  • #player1 doesn't need to be used at all. Put it inside the play button with the SVG code, and you can then more easily find it, and acces sit without needing any unique identifier.
  • You access the pay/pause buttons too many times. Assign them to variables and use those variables instead.

Accessing the DOM is expensive in terms of time for the browser to make the request, so reducing how much the DOM is accessed results in big performance gains.

style.display is bad - stop using it. Use classList.add/remove instead.

With the second code, the above information is all relevant to that code too.

Bad function name. firstClick might tell you what happens, but tells you nothing about what that function is supposed to do. That's as bad as using a "red" classname, because when the style is changed to be orange instead, you then have a "red" class that makes things orange. The function name of showLinks would be much more appropriate than firstClick.

Also, the second code assigns an event listener before the function. The code related to adding the event listener should appear below the function. It is preferable to have the functions grouped together, with all of the other code organised after all of the functions.


#74

Let's start with the 1st code:

<script>
  (function iife() {
    "use strict";
    document.querySelector(".myLink1").classList.add("hide");

    function playButtonClickHandler() {
      document.querySelector(".myLink1").classList.remove("hide");
      var button = document.querySelector(".playButton1");
      var player = document.querySelector("#player1");
      document.querySelector(".playButton1 .initial1").style.display = "none";
      player.volume = 1.0;
      if (player.paused) {
        button.classList.add("playing");
        document.querySelector(".playButton1 .play1").style.display = "none";
        document.querySelector(".playButton1 .pause1").style.display = "inline-block";
        player.play();
      } else {
        document.querySelector(".playButton1 .play1").style.display = "inline-block";
        document.querySelector(".playButton1 .pause1").style.display = "none";
        player.pause();
      }
    }
    var playButton = document.querySelector(".playButton1");
    playButton.addEventListener("click", playButtonClickHandler);
  }());

</script>

#75

We can start with how to do this first.
This will help with all my other codes too.

I don't understand how you do this. You mentioned this in another post too.


#76

Here is how you assign a reference to an HTML element to a variable.

var playButton = document.querySelector(".playButton");

From then on, you can use playButton to refer to that HTML element. For example, to get the <audio> element that's in the playButton element.

var player = playButton.querySelector("audio");

#77

Like that?

<script>
  (function iife() {
    "use strict";
    document.querySelector(".myLink1").classList.add("hide");

    function playButtonClickHandler() {
      document.querySelector(".myLink1").classList.remove("hide");
      var button = document.querySelector(".playButton1");
      var player = document.querySelector("#player1");
      document.querySelector(".initial1").style.display = "none";
      player.volume = 1.0;
      if (player.paused) {
        button.classList.add("playing");
        document.querySelector(".play1").style.display = "none";
        document.querySelector(".pause1").style.display = "inline-block";
        player.play();
      } else {
        document.querySelector(".play1").style.display = "inline-block";
        document.querySelector(".pause1").style.display = "none";
        player.pause();
      }
    }
    var playButton = document.querySelector(".playButton1");
    playButton.addEventListener("click", playButtonClickHandler);
  }());

</script>

#78

Replace
.style.display = "none";

with this
.classList.remove("hide");

Replace
.style.display = "inline-block";

with this
.classList.add("inline-block");


#79

Like what? I am not wasting time investigating and doing a line-by-line comparison with the many versions of your other code today. I am not interpreting signs from chicken gizzards today.

Use your words, and tell me the changes that you made to your code.


#80

this

document.querySelector(".playButton1 .initial1").style.display = "none";
document.querySelector(".playButton1 .play1").style.display = "none";
document.querySelector(".playButton1 .pause1").style.display = "inline-block";
document.querySelector(".playButton1 .play1").style.display = "inline-block";
document.querySelector(".playButton1 .pause1").style.display = "none";

to this

document.querySelector(".initial1").style.display = "none";
document.querySelector(".play1").style.display = "none";
document.querySelector(".pause1").style.display = "inline-block";
document.querySelector(".play1").style.display = "inline-block";
document.querySelector(".pause1").style.display = "none";

#81

No, the code still has "document" as the base instead of the ".playButton1" element as the base.


#82

like this?

playButton1.querySelector(".initial1").style.display = "none";
playButton1.querySelector(".play1").style.display = "none";
playButton1.querySelector(".pause1").style.display = "inline-block";
playButton1.querySelector(".play1").style.display = "inline-block";
playButton1.querySelector(".pause1").style.display = "none";

#83

Close, but the base name used needs to be the same as the variable name used to reference what was selected

var playButton = document.querySelector(".playButton1");

#84

That goes in this part?

I'm way confused.

  function playButtonClickHandler() {
      document.querySelector(".myLink1").classList.remove("hide");
      var button = document.querySelector(".playButton1");
      var player = document.querySelector("#player1");
      document.querySelector(".initial1").style.display = "none";
      player.volume = 1.0;
      if (player.paused) {

#85

While that's better, there's no need for the individual numbers on the class names, and most of those class names can be removed too.

Any CSS developer seeing those numbered class names will want to strangle you, because your techniques are extremely terrible, even if you don't know that they are terrible.


#86

That's already in the code.

var playButton = document.querySelector(".playButton1");
playButton.addEventListener("click", playButtonClickHandler);

}());


#87

If I'm going to have multiple audio codes on the same page, they all can't say playButton, unless you're telling me they can?

How will they all be able to differentiate themselves from other players on the page?

After I do this right, I'll use the same technique in others.